diff --git a/docs/content/faq.md b/docs/content/faq.md index d7f1877281..c10af37785 100644 --- a/docs/content/faq.md +++ b/docs/content/faq.md @@ -196,7 +196,7 @@ Use `-D` to force-delete branches with unmerged changes. Use `--no-delete-branch - `wt remove` — besides the target worktree, two cleanup mechanisms run. The removed worktree's own `git fsmonitor--daemon` (git's per-worktree filesystem watcher under `core.fsmonitor=true`, which would leak once its worktree is gone) is sent `git fsmonitor--daemon stop`, then force-terminated (`SIGTERM`, then `SIGKILL`) via the PID resolved from its IPC socket if it didn't exit. A background sweep then deletes `.git/wt/trash/` entries older than 24 hours (directories orphaned when a previous background removal was interrupted) and terminates fsmonitor daemons whose worktree no longer exists (orphans from `git worktree remove`, `rm -rf`, or a crashed `wt`) - `wt config state clear` — removes all worktrunk data from `.git/` (config keys, caches, markers, hints, variables, logs, stale trash) - `wt config shell install` — when migrating an integration to a new location, removes the worktrunk-managed file left at the old one: fish `conf.d/wt.fish` (now `functions/wt.fish`) and nushell wrappers stranded under `/vendor/autoload` (now `/vendor/autoload`) -- `wt config shell uninstall` — removes shell integration from rc files +- `wt config shell uninstall` — removes integration lines from bash/zsh/PowerShell rc files, and deletes worktrunk's wrapper and completion files (fish `functions/`, `conf.d/`, and `completions/`; nushell `vendor/autoload`). Files are recognized by worktrunk's own content markers, whatever binary name they were installed under; files without the markers are left alone See [What files does Worktrunk create?](#what-files-does-worktrunk-create) for details. diff --git a/plugins/worktrunk/skills/worktrunk/reference/faq.md b/plugins/worktrunk/skills/worktrunk/reference/faq.md index aa412f05ae..638c2004b7 100644 --- a/plugins/worktrunk/skills/worktrunk/reference/faq.md +++ b/plugins/worktrunk/skills/worktrunk/reference/faq.md @@ -191,7 +191,7 @@ Use `-D` to force-delete branches with unmerged changes. Use `--no-delete-branch - `wt remove` — besides the target worktree, two cleanup mechanisms run. The removed worktree's own `git fsmonitor--daemon` (git's per-worktree filesystem watcher under `core.fsmonitor=true`, which would leak once its worktree is gone) is sent `git fsmonitor--daemon stop`, then force-terminated (`SIGTERM`, then `SIGKILL`) via the PID resolved from its IPC socket if it didn't exit. A background sweep then deletes `.git/wt/trash/` entries older than 24 hours (directories orphaned when a previous background removal was interrupted) and terminates fsmonitor daemons whose worktree no longer exists (orphans from `git worktree remove`, `rm -rf`, or a crashed `wt`) - `wt config state clear` — removes all worktrunk data from `.git/` (config keys, caches, markers, hints, variables, logs, stale trash) - `wt config shell install` — when migrating an integration to a new location, removes the worktrunk-managed file left at the old one: fish `conf.d/wt.fish` (now `functions/wt.fish`) and nushell wrappers stranded under `/vendor/autoload` (now `/vendor/autoload`) -- `wt config shell uninstall` — removes shell integration from rc files +- `wt config shell uninstall` — removes integration lines from bash/zsh/PowerShell rc files, and deletes worktrunk's wrapper and completion files (fish `functions/`, `conf.d/`, and `completions/`; nushell `vendor/autoload`). Files are recognized by worktrunk's own content markers, whatever binary name they were installed under; files without the markers are left alone See [What files does Worktrunk create?](#what-files-does-worktrunk-create) for details. diff --git a/skills/worktrunk/reference/faq.md b/skills/worktrunk/reference/faq.md index aa412f05ae..638c2004b7 100644 --- a/skills/worktrunk/reference/faq.md +++ b/skills/worktrunk/reference/faq.md @@ -191,7 +191,7 @@ Use `-D` to force-delete branches with unmerged changes. Use `--no-delete-branch - `wt remove` — besides the target worktree, two cleanup mechanisms run. The removed worktree's own `git fsmonitor--daemon` (git's per-worktree filesystem watcher under `core.fsmonitor=true`, which would leak once its worktree is gone) is sent `git fsmonitor--daemon stop`, then force-terminated (`SIGTERM`, then `SIGKILL`) via the PID resolved from its IPC socket if it didn't exit. A background sweep then deletes `.git/wt/trash/` entries older than 24 hours (directories orphaned when a previous background removal was interrupted) and terminates fsmonitor daemons whose worktree no longer exists (orphans from `git worktree remove`, `rm -rf`, or a crashed `wt`) - `wt config state clear` — removes all worktrunk data from `.git/` (config keys, caches, markers, hints, variables, logs, stale trash) - `wt config shell install` — when migrating an integration to a new location, removes the worktrunk-managed file left at the old one: fish `conf.d/wt.fish` (now `functions/wt.fish`) and nushell wrappers stranded under `/vendor/autoload` (now `/vendor/autoload`) -- `wt config shell uninstall` — removes shell integration from rc files +- `wt config shell uninstall` — removes integration lines from bash/zsh/PowerShell rc files, and deletes worktrunk's wrapper and completion files (fish `functions/`, `conf.d/`, and `completions/`; nushell `vendor/autoload`). Files are recognized by worktrunk's own content markers, whatever binary name they were installed under; files without the markers are left alone See [What files does Worktrunk create?](#what-files-does-worktrunk-create) for details. diff --git a/src/cli/config.rs b/src/cli/config.rs index 635fd6aae3..a881e640a0 100644 --- a/src/cli/config.rs +++ b/src/cli/config.rs @@ -127,8 +127,8 @@ $ wt config shell uninstall --yes ## Version tolerance -Detects various forms of the integration pattern regardless of: -- Command prefix (wt, worktree, etc.) +Uninstall removes every worktrunk-managed integration it finds, regardless of: +- Binary name it was installed under (`wt`, `git-wt`, …) - Minor syntax variations between versions"# )] Uninstall { diff --git a/src/commands/configure_shell.rs b/src/commands/configure_shell.rs index 266362ffea..04ff6bebd1 100644 --- a/src/commands/configure_shell.rs +++ b/src/commands/configure_shell.rs @@ -124,15 +124,45 @@ fn is_worktrunk_managed_content(content: &str, cmd: &str) -> bool { content.contains(&format!("{cmd} config shell init")) && content.contains("| source") } +/// The header comment every wrapper template worktrunk has shipped opens with, +/// and which a user's own `wt.fish` / `wt.nu` would not carry. +const WRAPPER_MARKER: &str = "worktrunk shell integration for"; + +/// The header every fish completion file worktrunk has shipped opens with. +const COMPLETION_MARKER: &str = "# worktrunk completions for"; + +/// Cmd-agnostic content check: matches a worktrunk-generated wrapper file +/// regardless of the binary name embedded in it. +/// +/// Used by `uninstall` to recognize wrapper files installed under any binary +/// name (e.g. `wt.fish`, `git-wt.fish`, `git-wt.nu`) when scanning the wrapper +/// directories. Every wrapper template worktrunk renders carries the header, so +/// that answers first. The fallback covers the legacy `conf.d/{cmd}.fish`, which +/// predates the templates and holds nothing but the init line: a file qualifies +/// only when every non-blank, non-comment line is an integration line +/// (per the rc-file line detector, keeping one definition of "an integration +/// line"). Uninstall walks directories the user owns, so a user's own file that +/// runs `wt config shell init` amid other code — or merely mentions it — must +/// survive. +fn is_worktrunk_managed_content_any_cmd(content: &str) -> bool { + if content.contains(WRAPPER_MARKER) { + return true; + } + let code_lines = fish_code_lines(content); + !code_lines.is_empty() + && code_lines + .into_iter() + .all(shell::is_shell_integration_line_for_uninstall_any_cmd) +} + /// Check if a Nushell wrapper file is worktrunk-managed. /// /// The Nushell wrapper is a complete autoload file (not a `source` line), so it -/// carries no `config shell init` marker. Every version worktrunk has shipped -/// opens with this header comment, which a user's own `wt.nu` would not — so it -/// is a safe signal that the file is ours to remove during stranded-file -/// cleanup (issue #2878). +/// carries no `config shell init` marker — the header comment is the whole +/// signal that the file is ours to remove during stranded-file cleanup +/// (issue #2878). fn is_worktrunk_managed_nushell(content: &str) -> bool { - content.contains("worktrunk shell integration for nushell") + content.contains(&format!("{WRAPPER_MARKER} nushell")) } /// Clean up legacy fish conf.d file after installing to functions/ @@ -251,6 +281,8 @@ pub fn handle_configure_shell( dry_run: bool, cmd: String, ) -> Result { + shell::validate_shell_command_name(&cmd)?; + // First, do a dry-run to see what would be changed let preview = scan_shell_configs(shell_filter, true, &cmd)?; @@ -406,6 +438,8 @@ pub fn scan_shell_configs( dry_run: bool, cmd: &str, ) -> Result { + shell::validate_shell_command_name(cmd)?; + // Iterate every supported shell. Shells the user doesn't have are filtered // out of the Skipped output by `is_installed()` below, matching how // bash/zsh/fish/nushell are handled. @@ -542,7 +576,7 @@ fn configure_shell_file( let reader = BufReader::new(file); - // Check for the exact conditional wrapper we would write + // Check for the canonical line and older/manual forms for this shell. for line in reader.lines() { let line = line.map_err(|e| { format!( @@ -552,8 +586,7 @@ fn configure_shell_file( ) })?; - // Canonical detection: check if the line matches exactly what we write - if line.trim() == config_line { + if is_install_shell_integration_line(&line, shell, cmd) { return Ok(Some(ConfigureResult { shell, path: path.to_path_buf(), @@ -643,10 +676,19 @@ fn configure_shell_file( } } -/// Extract non-comment, non-blank lines from fish source for comparison. +fn is_install_shell_integration_line(line: &str, shell: Shell, cmd: &str) -> bool { + shell::is_shell_integration_line(line, cmd) + && line + .to_ascii_lowercase() + .contains(&format!("config shell init {shell}")) +} + +/// Extract non-comment, non-blank lines from fish/nushell source. /// -/// This lets us detect existing installations even when comment text has changed -/// between versions (e.g. updated documentation URLs). +/// Install compares wrapper code lines so it detects existing installations even +/// when comment text has changed between versions (e.g. updated documentation +/// URLs); uninstall classifies a headerless file by whether its code lines are +/// all integration lines. fn fish_code_lines(source: &str) -> Vec<&str> { source .lines() @@ -989,10 +1031,9 @@ pub fn handle_unconfigure_shell( shell_filter: Option, skip_confirmation: bool, dry_run: bool, - cmd: &str, ) -> Result { // First, do a dry-run to see what would be changed - let preview = scan_for_uninstall(shell_filter, true, cmd)?; + let preview = scan_for_uninstall(shell_filter, true)?; // If nothing to do, return early if preview.results.is_empty() && preview.completion_results.is_empty() { @@ -1021,7 +1062,7 @@ pub fn handle_unconfigure_shell( } // User confirmed (or --yes flag was used), now actually apply the changes - scan_for_uninstall(shell_filter, false, cmd) + scan_for_uninstall(shell_filter, false) } /// Remove a config file with a context-rich error message. @@ -1030,10 +1071,18 @@ fn remove_config_file(path: &std::path::Path) -> Result<(), String> { .map_err(|e| format!("Failed to remove {}: {e}", format_path_for_display(path))) } +/// Uninstall scans the shell-owned directories and removes every worktrunk-managed +/// file or line, regardless of the binary name it was installed under. +/// +/// For Fish/Nushell wrapper files and Fish completion files (one file per binary +/// name), this lists the owning directories and admits any file whose content +/// matches the worktrunk marker (`is_worktrunk_managed_content_any_cmd`, or the +/// completion header for `completions/`). For Bash/Zsh/PowerShell (line-based), +/// it scans the rc/profile files and uses `is_shell_integration_line_for_uninstall_any_cmd`. +/// No `--cmd` is needed because the marker is the content, not the file name. fn scan_for_uninstall( shell_filter: Option, dry_run: bool, - cmd: &str, ) -> Result { // For uninstall, scan every shell (Shell::all includes PowerShell) to clean // up any existing profiles. @@ -1041,172 +1090,142 @@ fn scan_for_uninstall( let shells = shell_filter.map_or(default_shells, |shell| vec![shell]); + let home = + shell::home_dir_required().map_err(|e| format!("Cannot determine home directory: {e}"))?; + let mut results = Vec::new(); let mut not_found = Vec::new(); for &shell in &shells { - let paths = shell - .config_paths(cmd) - .map_err(|e| format!("Failed to get config paths for {shell}: {e}"))?; + match shell { + Shell::Fish => { + let functions_dir = home.join(".config").join("fish").join("functions"); + let confd_dir = home.join(".config").join("fish").join("conf.d"); + + let canonical = scan_managed_files( + &functions_dir, + "fish", + is_worktrunk_managed_content_any_cmd, + )?; + let legacy = + scan_managed_files(&confd_dir, "fish", is_worktrunk_managed_content_any_cmd)?; + let found_any = !canonical.is_empty() || !legacy.is_empty(); + + for path in &canonical { + let action = if dry_run { + UninstallAction::WouldRemove + } else { + remove_config_file(path)?; + UninstallAction::Removed + }; + results.push(UninstallResult { + shell, + path: path.clone(), + action, + superseded_by: None, + }); + } - // For Fish, delete entire {cmd}.fish file (check both canonical and legacy locations) - if matches!(shell, Shell::Fish) { - let mut found_any = false; - - // Check canonical location (functions/) - // Only remove if it contains worktrunk markers to avoid deleting user's custom file - if let Some(fish_path) = paths.first() - && fish_path.exists() - { - let is_worktrunk_managed = fs::read_to_string(fish_path) - .map(|content| is_worktrunk_managed_content(&content, cmd)) - .unwrap_or(false); - - if is_worktrunk_managed { - found_any = true; - if dry_run { - results.push(UninstallResult { - shell, - path: fish_path.clone(), - action: UninstallAction::WouldRemove, - superseded_by: None, - }); + for path in &legacy { + let superseded_by = path.file_name().map(|n| functions_dir.join(n)); + let action = if dry_run { + UninstallAction::WouldRemove } else { - remove_config_file(fish_path)?; - results.push(UninstallResult { - shell, - path: fish_path.clone(), - action: UninstallAction::Removed, - superseded_by: None, - }); - } + remove_config_file(path)?; + UninstallAction::Removed + }; + results.push(UninstallResult { + shell, + path: path.clone(), + action, + superseded_by, + }); + } + + if !found_any { + not_found.push((shell, functions_dir)); } } - // Also check legacy location (conf.d/) - issue #566 - // Only remove if it contains worktrunk markers to avoid deleting user's custom file - let canonical_path = paths.first().cloned(); - if let Ok(legacy_path) = Shell::legacy_fish_conf_d_path(cmd) - && legacy_path.exists() - { - let is_worktrunk_managed = fs::read_to_string(&legacy_path) - .map(|content| is_worktrunk_managed_content(&content, cmd)) - .unwrap_or(false); - - if is_worktrunk_managed { - found_any = true; - if dry_run { + Shell::Nushell => { + let mut found_any = false; + let candidates = shell::nushell_autoload_candidates(&home); + for autoload_dir in &candidates { + let nu_files = scan_managed_files( + autoload_dir, + "nu", + is_worktrunk_managed_content_any_cmd, + )?; + for path in &nu_files { + found_any = true; + let action = if dry_run { + UninstallAction::WouldRemove + } else { + remove_config_file(path)?; + UninstallAction::Removed + }; results.push(UninstallResult { shell, - path: legacy_path.clone(), - action: UninstallAction::WouldRemove, - superseded_by: canonical_path.clone(), - }); - } else { - remove_config_file(&legacy_path)?; - results.push(UninstallResult { - shell, - path: legacy_path, - action: UninstallAction::Removed, - superseded_by: canonical_path, + path: path.clone(), + action, + superseded_by: None, }); } } - } - - if !found_any && let Some(fish_path) = paths.first() { - not_found.push((shell, fish_path.clone())); - } - continue; - } - - // For Nushell, delete config files from all candidate locations. - // Installation might have written to a different path than what we'd pick now - // (e.g., `nu` was in PATH during install but not during uninstall). - if matches!(shell, Shell::Nushell) { - let mut found_any = false; - for config_path in &paths { - if !config_path.exists() { - continue; - } - found_any = true; - if dry_run { - results.push(UninstallResult { - shell, - path: config_path.clone(), - action: UninstallAction::WouldRemove, - superseded_by: None, - }); - } else { - remove_config_file(config_path)?; - results.push(UninstallResult { - shell, - path: config_path.clone(), - action: UninstallAction::Removed, - superseded_by: None, - }); + if !found_any { + // Report the first candidate's autoload dir as the expected location + if let Some(first) = candidates.first() { + not_found.push((shell, first.clone())); + } } } - if !found_any && let Some(config_path) = paths.first() { - not_found.push((shell, config_path.clone())); - } - continue; - } - // For Bash/Zsh, scan config files - let mut found = false; + Shell::Bash | Shell::Zsh | Shell::PowerShell => { + let paths = shell::line_based_config_paths(shell, &home); + let mut found = false; - for path in &paths { - if !path.exists() { - continue; - } + for path in &paths { + if !path.exists() { + continue; + } - match uninstall_from_file(shell, path, dry_run, cmd) { - Ok(Some(result)) => { - results.push(result); - found = true; - break; // Only process first matching file per shell + if let Some(result) = uninstall_from_file(shell, path, dry_run)? { + results.push(result); + found = true; + } } - Ok(None) => {} // No integration found in this file - Err(e) => return Err(e), - } - } - if !found && let Some(first_path) = paths.first() { - not_found.push((shell, first_path.clone())); + if !found && let Some(first_path) = paths.first() { + not_found.push((shell, first_path.clone())); + } + } } } - // Fish has a separate completion file that needs to be removed + // Fish completion files carry their own header marker, so they are scanned + // the same way as the wrappers — a completion whose wrapper is already gone + // is still cleaned up. let mut completion_results = Vec::new(); let mut completion_not_found = Vec::new(); - - for &shell in &shells { - if shell != Shell::Fish { - continue; + if shells.contains(&Shell::Fish) { + let completions_dir = home.join(".config").join("fish").join("completions"); + let completions = + scan_managed_files(&completions_dir, "fish", |c| c.contains(COMPLETION_MARKER))?; + if completions.is_empty() { + completion_not_found.push((Shell::Fish, completions_dir)); } - - let completion_path = shell - .completion_path(cmd) - .map_err(|e| format!("Failed to get completion path for {}: {}", shell, e))?; - - if completion_path.exists() { - if dry_run { - completion_results.push(CompletionUninstallResult { - shell, - path: completion_path, - action: UninstallAction::WouldRemove, - }); + for path in completions { + let action = if dry_run { + UninstallAction::WouldRemove } else { - remove_config_file(&completion_path)?; - completion_results.push(CompletionUninstallResult { - shell, - path: completion_path, - action: UninstallAction::Removed, - }); - } - } else { - completion_not_found.push((shell, completion_path)); + remove_config_file(&path)?; + UninstallAction::Removed + }; + completion_results.push(CompletionUninstallResult { + shell: Shell::Fish, + path, + action, + }); } } @@ -1218,11 +1237,45 @@ fn scan_for_uninstall( }) } +/// List `*.{extension}` files in `dir` whose content `is_managed` accepts. +/// Returns empty if `dir` does not exist; sorted so removal order is stable. +fn scan_managed_files( + dir: &Path, + extension: &str, + is_managed: fn(&str) -> bool, +) -> Result, String> { + if !dir.exists() { + return Ok(Vec::new()); + } + let entries = fs::read_dir(dir) + .map_err(|e| format!("Failed to read {}: {e}", format_path_for_display(dir)))?; + let mut out = Vec::new(); + for entry in entries { + let entry = + entry.map_err(|e| format!("Failed to read {}: {e}", format_path_for_display(dir)))?; + let path = entry.path(); + if path.extension().and_then(|e| e.to_str()) != Some(extension) { + continue; + } + if !path.is_file() { + continue; + } + let content = match fs::read_to_string(&path) { + Ok(s) => s, + Err(_) => continue, // unreadable file: skip, don't fail the whole uninstall + }; + if is_managed(&content) { + out.push(path); + } + } + out.sort(); + Ok(out) +} + fn uninstall_from_file( shell: Shell, path: &Path, dry_run: bool, - cmd: &str, ) -> Result, String> { let content = fs::read_to_string(path) .map_err(|e| format!("Failed to read {}: {}", format_path_for_display(path), e))?; @@ -1231,7 +1284,7 @@ fn uninstall_from_file( let integration_lines: Vec<(usize, &str)> = lines .iter() .enumerate() - .filter(|(_, line)| shell::is_shell_integration_line_for_uninstall(line, cmd)) + .filter(|(_, line)| shell::is_shell_integration_line_for_uninstall_any_cmd(line)) .map(|(i, line)| (i, *line)) .collect(); @@ -1564,4 +1617,72 @@ mod tests { let new = "# Docs: https://worktrunk.dev/config/#shell-integration\nfunction wt\n command wt $argv\nend"; assert_eq!(fish_code_lines(old), fish_code_lines(new)); } + + #[test] + fn test_managed_content_any_cmd_matches_wrappers_not_mentions() { + // Ours: the header every wrapper template carries, under any binary + // name, plus the headerless legacy `conf.d/{cmd}.fish`, whose code + // lines are nothing but the init invocation. + for content in [ + "# worktrunk shell integration for fish\nfunction git-wt\nend\n", + "# worktrunk shell integration for nushell\ndef --env wt [] {}\n", + "wt config shell init fish | source", + "# added by worktrunk\nif type -q wt; command wt config shell init fish | source; end\n", + ] { + assert!(is_worktrunk_managed_content_any_cmd(content), "{content}"); + } + + // Not ours: uninstall walks directories the user owns, so a file that + // only mentions the command must survive — as must a user's own file + // that runs the init line amid other code, which a per-line any-match + // would delete whole. + for content in [ + "function notes\n echo run wt config shell init fish to set up\nend\n", + "# reminder: wt config shell init fish\nfunction helpers\n cat ~/.aliases | source\nend\n", + "function helper\n command wt config shell init fish | source\nend\n", + "", + ] { + assert!(!is_worktrunk_managed_content_any_cmd(content), "{content}"); + } + } + + #[test] + fn test_line_based_config_paths_fish_and_nushell_have_no_line_files() { + // Fish and Nushell are configured via wrapper files, not line-based rc + // edits, so the line-based path list is empty for them. + let home = Path::new("/home/user"); + assert!(shell::line_based_config_paths(Shell::Fish, home).is_empty()); + assert!(shell::line_based_config_paths(Shell::Nushell, home).is_empty()); + } + + #[test] + fn test_scan_managed_files_skips_wrong_extension_and_directories() { + let dir = tempfile::TempDir::new().unwrap(); + let root = dir.path(); + // Wrong extension: skipped. + fs::write(root.join("notes.txt"), "anything").unwrap(); + // A directory carrying the wrapper extension: skipped (not a file). + fs::create_dir(root.join("nested.fish")).unwrap(); + assert!( + scan_managed_files(root, "fish", is_worktrunk_managed_content_any_cmd) + .unwrap() + .is_empty() + ); + } + + #[cfg(unix)] + #[test] + fn test_scan_managed_files_skips_unreadable_file() { + use std::os::unix::fs::PermissionsExt; + let dir = tempfile::TempDir::new().unwrap(); + let root = dir.path(); + let unreadable = root.join("locked.fish"); + fs::write(&unreadable, "function wt\nend\n").unwrap(); + fs::set_permissions(&unreadable, fs::Permissions::from_mode(0o000)).unwrap(); + // An unreadable wrapper file is skipped, not surfaced as an error. + let found = scan_managed_files(root, "fish", is_worktrunk_managed_content_any_cmd); + // Restore perms so TempDir cleanup can remove the file. + fs::set_permissions(&unreadable, fs::Permissions::from_mode(0o644)).unwrap(); + assert!(found.unwrap().is_empty()); + } } diff --git a/src/commands/init.rs b/src/commands/init.rs index fb011e5832..d619e94448 100644 --- a/src/commands/init.rs +++ b/src/commands/init.rs @@ -7,6 +7,8 @@ use worktrunk::shell; use worktrunk::styling::println; pub fn handle_init(shell: shell::Shell, cmd: String) -> Result<(), String> { + shell::validate_shell_command_name(&cmd)?; + let init = shell::ShellInit::with_prefix(shell, cmd); // Generate shell integration code (includes dynamic completion registration) @@ -50,6 +52,7 @@ pub fn handle_init(shell: shell::Shell, cmd: String) -> Result<(), String> { /// - Include shell integration (cd-on-switch functionality) pub fn handle_completions(shell: shell::Shell) -> anyhow::Result<()> { let cmd_name = crate::binary_name(); + shell::validate_shell_command_name(&cmd_name).map_err(anyhow::Error::msg)?; let mut stdout = io::stdout(); match shell { diff --git a/src/main.rs b/src/main.rs index c221a43bac..9ced3365ba 100644 --- a/src/main.rs +++ b/src/main.rs @@ -610,7 +610,7 @@ fn handle_config_shell_command(action: ConfigShellCommand, yes: bool) -> anyhow: } ConfigShellCommand::Uninstall { shell, dry_run } => { let explicit_shell = shell.is_some(); - handle_unconfigure_shell(shell, yes, dry_run, &binary_name()) + handle_unconfigure_shell(shell, yes, dry_run) .map_err(|e| anyhow::anyhow!("{}", e)) .map(|result| { if !dry_run { diff --git a/src/shell/detection.rs b/src/shell/detection.rs index a0b367bde2..0bbc7e5c2b 100644 --- a/src/shell/detection.rs +++ b/src/shell/detection.rs @@ -85,47 +85,113 @@ use super::paths::{home_dir_required, powershell_profile_paths}; /// This means false negatives only cause incorrect messaging in `wt config show` /// and when users run the binary directly before restarting their shell. pub fn is_shell_integration_line(line: &str, cmd: &str) -> bool { - is_shell_integration_line_impl(line, cmd, true) + has_init_invocation(line_code_portion(line), cmd) } -/// Permissive version for uninstall - matches old PowerShell configs without `| Out-String`. +/// The code portion of a config line: empty for comment lines, otherwise the +/// text before any trailing comment — a comment can mention an integration line +/// without running it, so detection reads only the code. /// -/// Used by `wt config shell uninstall` to find and remove outdated config lines -/// that would otherwise be left behind. -pub fn is_shell_integration_line_for_uninstall(line: &str, cmd: &str) -> bool { - is_shell_integration_line_impl(line, cmd, false) +/// A `#` opens a comment only at a word start (line start or after whitespace); +/// `$#` and PowerShell's `<#` stay code. Quote-blind by design: no integration +/// form worktrunk writes contains `#`. +fn line_code_portion(line: &str) -> &str { + let trimmed = line.trim(); + // Comment lines (# for POSIX shells, <# #> for PowerShell block comments) + if trimmed.starts_with('#') || trimmed.starts_with("<#") { + return ""; + } + let end = trimmed + .char_indices() + .find(|&(i, c)| c == '#' && trimmed[..i].ends_with(char::is_whitespace)) + .map_or(trimmed.len(), |(i, _)| i); + &trimmed[..end] } -fn is_shell_integration_line_impl(line: &str, cmd: &str, strict: bool) -> bool { - let trimmed = line.trim(); +/// The invariant part of every integration line. Only the binary name in front +/// of it varies, so it anchors both the cmd-specific and cmd-agnostic searches. +const INIT_MARKER: &str = " config shell init"; - // Skip comments (# for POSIX shells, <# #> for PowerShell block comments) - if trimmed.starts_with('#') || trimmed.starts_with("<#") { - return false; +/// Cmd-agnostic line detection: matches an integration line whatever binary name +/// it was installed under. +/// +/// Where the cmd-specific detectors search for a known name and check what +/// precedes it, this reads the name off the line: it is the run of command-name +/// characters ending at `INIT_MARKER`. Holding that run to +/// [`validate_shell_command_name`](super::validate_shell_command_name) — the +/// same rule governing what worktrunk is willing to write into shell code — +/// keeps detection and generation from drifting apart. +/// +/// Unlike the cmd-specific detectors this deliberately does not exclude a `git ` +/// prefix: `git wt config shell init` is worktrunk's own integration too, and +/// uninstall asks only whether a line is ours, not which install it belongs to. +/// +/// Used by `wt config shell uninstall`, which removes every worktrunk-managed +/// integration it finds. +pub fn is_shell_integration_line_for_uninstall_any_cmd(line: &str) -> bool { + let code = line_code_portion(line); + is_execution_context(code, false) + && code + .match_indices(INIT_MARKER) + .any(|(pos, _)| command_name_precedes(code, pos)) +} + +/// Does a plausible command name end at `pos`? +/// +/// Reads backwards over command-name characters, which stops at the shell +/// punctuation wrapping the invocation (`"$(`, `<(`) and at the directory +/// separator of an absolute path, leaving the bare name. An empty run means the +/// marker stands alone with nothing invoking it. +fn command_name_precedes(line: &str, pos: usize) -> bool { + let before = &line[..pos]; + before + .char_indices() + .rev() + .take_while(|(_, c)| c.is_ascii_alphanumeric() || matches!(c, '.' | '_' | '-')) + .map(|(i, _)| i) + .last() + .is_some_and(|start| super::validate_shell_command_name(&before[start..]).is_ok()) +} + +/// Does the line run the command's output, rather than merely mention it? +/// +/// A property of the whole line, not of where the command sits within it. +/// +/// PowerShell is checked first, case-insensitively. It needs `| Out-String` to +/// work — without it `Invoke-Expression` fails with "Cannot convert +/// 'System.Object[]'" (issue #885). Strict mode (the cmd-specific install-side +/// detector) leaves an old config lacking it undetected so `wt config shell +/// install` will rewrite it; the cmd-agnostic uninstall detector passes +/// `strict: false` so the old line can be removed. +fn is_execution_context(line: &str, strict: bool) -> bool { + let line_lower = line.to_lowercase(); + if line_lower.contains("invoke-expression") || line_lower.contains("iex") { + return !strict || line_lower.contains("out-string"); } - // Check for eval/source line pattern - has_init_invocation(trimmed, cmd, strict) + // POSIX shells (bash, zsh, fish) and nushell + line.contains("eval") + || line.contains("source") + || line.contains(". <(") // POSIX dot command with process substitution + || line.contains(". =(") // zsh dot command with =() substitution + || line.contains("save") // nushell pipe to save } /// Check if line contains `{cmd} config shell init` as a command invocation. /// /// For `wt`: matches `wt config shell init` but NOT `git wt` or `git-wt`. /// For `git-wt`: matches `git-wt config shell init` OR `git wt config shell init`. -/// -/// When `strict` is true, PowerShell lines must include `| Out-String` to match. -/// When `strict` is false (for uninstall), old PowerShell lines without it also match. -fn has_init_invocation(line: &str, cmd: &str, strict: bool) -> bool { +fn has_init_invocation(line: &str, cmd: &str) -> bool { // For git-wt, we need to match both "git-wt config shell init" AND "git wt config shell init" // because users invoke it both ways (and git dispatches "git wt" to "git-wt") if cmd == "git-wt" { // Match either form, with boundary check for "git" in "git wt" form - return has_init_pattern_with_prefix_check(line, "git-wt", strict) - || has_init_pattern_with_prefix_check(line, "git wt", strict); + return has_init_pattern_with_prefix_check(line, "git-wt") + || has_init_pattern_with_prefix_check(line, "git wt"); } // For other commands, use normal matching with prefix exclusion - has_init_pattern_with_prefix_check(line, cmd, strict) + has_init_pattern_with_prefix_check(line, cmd) } /// Check if line has the init pattern, with prefix exclusion for non-git-wt commands. @@ -135,63 +201,22 @@ fn has_init_invocation(line: &str, cmd: &str, strict: bool) -> bool { /// ```text /// eval "$(git-wt.exe config shell init bash)" /// ``` -/// -/// When `strict` is true, PowerShell lines must include `| Out-String`. -/// When `strict` is false (for uninstall), old PowerShell lines also match. -fn has_init_pattern_with_prefix_check(line: &str, cmd: &str, strict: bool) -> bool { - // Search for both plain command and .exe variant (Windows Git Bash) - let patterns = [ - format!("{cmd} config shell init"), - format!("{cmd}.exe config shell init"), - ]; +fn has_init_pattern_with_prefix_check(line: &str, cmd: &str) -> bool { + if !is_execution_context(line, true) { + return false; + } - for init_pattern in &patterns { - // Determine the command portion for position checking - // For ".exe" pattern, the command in the line includes ".exe" - let cmd_in_line = if init_pattern.contains(".exe") { - format!("{cmd}.exe") - } else { - cmd.to_string() - }; + // Search for both plain command and .exe variant (Windows Git Bash) + for cmd_in_line in [cmd.to_string(), format!("{cmd}.exe")] { + let init_pattern = format!("{cmd_in_line}{INIT_MARKER}"); let mut search_start = 0; - while let Some(pos) = line[search_start..].find(init_pattern.as_str()) { + while let Some(pos) = line[search_start..].find(&init_pattern) { let absolute_pos = search_start + pos; // Check what precedes the match if is_valid_command_position(line, absolute_pos, &cmd_in_line) { - // Must be in an execution context (eval, source, dot command, PowerShell, etc.) - // - // PowerShell detection is checked FIRST and uses case-insensitive matching. - // PowerShell requires | Out-String to work correctly (issue #885). - // Without it, Invoke-Expression fails with "Cannot convert 'System.Object[]'". - // In strict mode, we don't detect old configs without Out-String so that - // `wt config shell install` will update them. - // In permissive mode (uninstall), we match old configs so they can be removed. - let line_lower = line.to_lowercase(); - let has_invoke = - line_lower.contains("invoke-expression") || line_lower.contains("iex"); - if has_invoke { - // PowerShell line - if !strict || line_lower.contains("out-string") { - return true; - } - // Strict mode: old PowerShell config without Out-String, don't detect - // Skip to next pattern search position - search_start = absolute_pos + 1; - continue; - } - - // POSIX shells (bash, zsh, fish) and nushell - let is_shell_exec = line.contains("eval") - || line.contains("source") - || line.contains(". <(") // POSIX dot command with process substitution - || line.contains(". =(") // zsh dot command with =() substitution - || line.contains("save"); // nushell pipe to save - - if is_shell_exec { - return true; - } + return true; } // Continue searching after this match @@ -763,37 +788,62 @@ mod tests { ); } - /// Permissive mode (for uninstall) SHOULD detect old PowerShell lines without | Out-String + /// A trailing comment can mention an integration line without running it; + /// detection reads only the code portion of the line. #[test] - fn test_powershell_permissive_mode_for_uninstall() { - // Old configs should be detected by the permissive function (for uninstall) - assert!( - is_shell_integration_line_for_uninstall("iex (wt config shell init powershell)", "wt"), - "Permissive mode should detect old PowerShell config" - ); - assert!( - is_shell_integration_line_for_uninstall( - "Invoke-Expression (& wt config shell init powershell)", - "wt" - ), - "Permissive mode should detect old Invoke-Expression config" - ); - // The exact old canonical line - assert!( - is_shell_integration_line_for_uninstall( - "if (Get-Command wt -ErrorAction SilentlyContinue) { Invoke-Expression (& wt config shell init powershell) }", - "wt" - ), - "Permissive mode should detect exact old canonical PowerShell line" - ); - // New configs should also be detected - assert!( - is_shell_integration_line_for_uninstall( - "iex (wt config shell init powershell | Out-String)", - "wt" - ), - "Permissive mode should also detect new PowerShell config" - ); + fn test_trailing_comment_is_not_code() { + // The comment mentions the init call; the code runs something else. + // Deleting this line would take the user's direnv hook with it. + let line = r#"eval "$(direnv hook bash)" # see: wt config shell init bash"#; + assert_not_detects(line, "wt", "mention in trailing comment"); + assert!(!is_shell_integration_line_for_uninstall_any_cmd(line)); + + // A real integration line keeps matching with a comment after it, + // and `$#` does not open a comment. + for line in [ + r#"eval "$(wt config shell init bash)" # worktrunk"#, + r#"if [ $# -eq 0 ]; then :; fi; eval "$(wt config shell init bash)""#, + ] { + assert_detects(line, "wt", "code before trailing comment"); + assert!(is_shell_integration_line_for_uninstall_any_cmd(line)); + } + } + + #[test] + fn test_any_cmd_detects_managed_line_regardless_of_binary_name() { + // The name is read off the line, so every form worktrunk writes matches, + // as does the `git wt` dispatch form a cmd-specific detector rejects. + for line in [ + r#"eval "$(wt config shell init bash)""#, + r#"eval "$(command git-wt config shell init zsh)""#, + r#"eval "$(git wt config shell init bash)""#, + r#"eval "$(git-wt.exe config shell init bash)""#, + r#"eval "$(/opt/homebrew/bin/my.tool_1 config shell init bash)""#, + "if type -q wt; command wt config shell init fish | source; end", + "iex (wt config shell init powershell | Out-String)", + // Old PowerShell forms without `| Out-String`, which uninstall + // must still remove (permissive execution-context check). + "iex (wt config shell init powershell)", + "if (Get-Command wt -ErrorAction SilentlyContinue) { Invoke-Expression (& wt config shell init powershell) }", + ] { + assert!( + is_shell_integration_line_for_uninstall_any_cmd(line), + "should detect: {line}" + ); + } + + // Not managed: a mention that never runs, a commented-out line, and a + // marker with no command in front of it. + for line in [ + "echo wt config shell init bash", + r#"# eval "$(wt config shell init bash)""#, + r#"eval "$( config shell init bash)""#, + ] { + assert!( + !is_shell_integration_line_for_uninstall_any_cmd(line), + "should not detect: {line}" + ); + } } // ------------------------------------------------------------------------ diff --git a/src/shell/mod.rs b/src/shell/mod.rs index 14a28b8e1a..330639263c 100644 --- a/src/shell/mod.rs +++ b/src/shell/mod.rs @@ -16,14 +16,42 @@ use askama::Template; // Re-export public types and functions pub use detection::{ BypassAlias, DetectedLine, FileDetectionResult, is_shell_integration_line, - is_shell_integration_line_for_uninstall, scan_for_detection_details, + is_shell_integration_line_for_uninstall_any_cmd, scan_for_detection_details, +}; +pub use paths::{ + completion_path, config_paths, home_dir_required, legacy_fish_conf_d_path, + line_based_config_paths, nushell_autoload_candidates, }; -pub use paths::{completion_path, config_paths, legacy_fish_conf_d_path}; pub use utils::{ AncestorShell, ancestor_shell, current_shell, current_shell_name, detect_zsh_compinit, extract_filename_from_path, }; +/// Validate a command name before embedding it in shell syntax or shell-owned paths. +pub fn validate_shell_command_name(cmd: &str) -> Result<(), String> { + if cmd.is_empty() { + return Err("Invalid shell integration command name: command name cannot be empty".into()); + } + + if cmd.starts_with('-') { + return Err( + "Invalid shell integration command name: command name cannot start with '-'".into(), + ); + } + + if !cmd + .bytes() + .all(|b| b.is_ascii_alphanumeric() || matches!(b, b'.' | b'_' | b'-')) + { + return Err( + "Invalid shell integration command name: use only ASCII letters, numbers, '.', '_', and '-'" + .into(), + ); + } + + Ok(()) +} + /// Supported shells /// /// Currently supported: bash, fish, nushell (experimental), zsh, powershell @@ -145,6 +173,9 @@ impl Shell { /// The `cmd` parameter specifies the command name (e.g., `wt` or `git-wt`). /// All shells use a conditional wrapper to avoid errors when the command doesn't exist. /// + /// `cmd` must already be validated by [`validate_shell_command_name`]; command + /// entry points (`handle_init`, `handle_configure_shell`) validate at the edge. + /// /// Note: The generated line does not include `--cmd` because `binary_name()` already /// detects the command name from argv\[0\] at runtime. pub fn config_line(&self, cmd: &str) -> String { @@ -223,6 +254,8 @@ pub struct ShellInit { } impl ShellInit { + /// `cmd` must already be validated by [`validate_shell_command_name`]; command + /// entry points (`handle_init`, `handle_configure_shell`) validate at the edge. pub fn with_prefix(shell: Shell, cmd: String) -> Self { Self { shell, cmd } } @@ -564,6 +597,28 @@ mod tests { insta::assert_snapshot!(init.generate().expect("Should generate with custom prefix")); } + #[rstest] + #[case("wt")] + #[case("git-wt")] + #[case("my.app_1")] + fn test_validate_shell_command_name_accepts_safe_names(#[case] cmd: &str) { + assert!(validate_shell_command_name(cmd).is_ok()); + } + + #[rstest] + #[case("")] + #[case("-wt")] + #[case("wt; touch")] + #[case("wt touch")] + #[case("wt/touch")] + #[case("wt\\touch")] + #[case("wt\ntouch")] + #[case("wt'touch")] + #[case("wüt")] + fn test_validate_shell_command_name_rejects_shell_syntax(#[case] cmd: &str) { + assert!(validate_shell_command_name(cmd).is_err()); + } + /// Verify that `config_line()` generates lines that /// `is_shell_integration_line()` can detect. /// diff --git a/src/shell/paths.rs b/src/shell/paths.rs index 0cb0aa80f8..2e6cb22dce 100644 --- a/src/shell/paths.rs +++ b/src/shell/paths.rs @@ -159,7 +159,7 @@ fn legacy_nushell_autoload_dirs( /// The first entry is the canonical write target (the current vendor-autoload /// dir); the rest are the data-dir fallback and the legacy config-dir locations /// kept so install/uninstall can clean up stranded files. -fn nushell_autoload_candidates(home: &std::path::Path) -> Vec { +pub fn nushell_autoload_candidates(home: &std::path::Path) -> Vec { let dirs = nu_dirs(); let mut candidates = vec![nushell_vendor_autoload_dir( home, @@ -209,14 +209,14 @@ pub fn powershell_profile_paths(home: &std::path::Path) -> Vec { } } -/// Returns the config file paths for a shell. +/// Rc/profile files scanned line-by-line for integration lines. /// -/// The `cmd` parameter affects the Fish functions filename (e.g., `wt.fish` or `git-wt.fish`). -/// Returns paths in order of preference. The first existing file should be used. -pub fn config_paths(shell: super::Shell, cmd: &str) -> Result, std::io::Error> { - let home = home_dir_required()?; - - Ok(match shell { +/// Bash/Zsh/PowerShell integration is one line in an rc file, so these paths +/// are independent of the binary name; Fish and Nushell use per-name wrapper +/// files instead and have no line-based config. `config_paths` builds on this +/// for the line-based shells, so install and uninstall cannot drift apart. +pub fn line_based_config_paths(shell: super::Shell, home: &std::path::Path) -> Vec { + match shell { super::Shell::Bash => { // Use .bashrc - sourced by interactive shells (login shells should source .bashrc) vec![home.join(".bashrc")] @@ -224,9 +224,25 @@ pub fn config_paths(shell: super::Shell, cmd: &str) -> Result, std: super::Shell::Zsh => { let zdotdir = std::env::var("ZDOTDIR") .map(PathBuf::from) - .unwrap_or_else(|_| home.clone()); + .unwrap_or_else(|_| home.to_path_buf()); vec![zdotdir.join(".zshrc")] } + super::Shell::PowerShell => powershell_profile_paths(home), + super::Shell::Fish | super::Shell::Nushell => Vec::new(), + } +} + +/// Returns the config file paths for a shell. +/// +/// The `cmd` parameter affects the Fish functions filename (e.g., `wt.fish` or `git-wt.fish`). +/// Returns paths in order of preference. The first existing file should be used. +pub fn config_paths(shell: super::Shell, cmd: &str) -> Result, std::io::Error> { + let home = home_dir_required()?; + + Ok(match shell { + super::Shell::Bash | super::Shell::Zsh | super::Shell::PowerShell => { + line_based_config_paths(shell, &home) + } super::Shell::Fish => { // For fish, we write to functions/ which is autoloaded on first use. // This ensures PATH is fully configured before our function loads, @@ -250,7 +266,6 @@ pub fn config_paths(shell: super::Shell, cmd: &str) -> Result, std: .map(|autoload_dir| autoload_dir.join(format!("{}.nu", cmd))) .collect() } - super::Shell::PowerShell => powershell_profile_paths(&home), }) } diff --git a/tests/integration_tests/configure_shell.rs b/tests/integration_tests/configure_shell.rs index f356280119..7ab6f3b074 100644 --- a/tests/integration_tests/configure_shell.rs +++ b/tests/integration_tests/configure_shell.rs @@ -88,6 +88,43 @@ fn test_configure_shell_specific_shell(repo: TestRepo, temp_home: TempDir) { assert!(content.contains("eval \"$(command wt config shell init zsh)\"")); } +#[rstest] +fn test_configure_shell_rejects_unsafe_cmd_without_modifying_rc( + repo: TestRepo, + temp_home: TempDir, +) { + let zshrc_path = temp_home.path().join(".zshrc"); + let original = "# Existing config\n"; + fs::write(&zshrc_path, original).unwrap(); + + let mut cmd = wt_command(); + repo.configure_wt_cmd(&mut cmd); + set_temp_home_env(&mut cmd, temp_home.path()); + cmd.arg("config") + .arg("shell") + .arg("install") + .arg("zsh") + .arg("--yes") + .arg("--cmd") + .arg("wt; touch /tmp/pwn") + .current_dir(repo.root_path()); + + let output = cmd.output().unwrap(); + + assert!(!output.status.success()); + assert!( + output.stdout.is_empty(), + "unsafe command name must not emit shell code:\n{}", + String::from_utf8_lossy(&output.stdout) + ); + assert!( + String::from_utf8_lossy(&output.stderr).contains("Invalid shell integration command name"), + "expected validation error, got:\n{}", + String::from_utf8_lossy(&output.stderr) + ); + assert_eq!(fs::read_to_string(&zshrc_path).unwrap(), original); +} + #[rstest] fn test_configure_shell_already_exists(repo: TestRepo, temp_home: TempDir) { // Create a fake .zshrc file with the line already present @@ -128,6 +165,39 @@ fn test_configure_shell_already_exists(repo: TestRepo, temp_home: TempDir) { assert_eq!(count, 1, "Should only have one wt config shell init line"); } +#[rstest] +fn test_configure_shell_already_exists_noncanonical_line(repo: TestRepo, temp_home: TempDir) { + let zshrc_path = temp_home.path().join(".zshrc"); + fs::write( + &zshrc_path, + "# Existing config\neval \"$(wt config shell init zsh)\"\n", + ) + .unwrap(); + + let mut cmd = wt_command(); + repo.configure_wt_cmd(&mut cmd); + set_temp_home_env(&mut cmd, temp_home.path()); + cmd.env("SHELL", "/bin/zsh"); + cmd.env("WORKTRUNK_TEST_COMPINIT_CONFIGURED", "1"); + cmd.arg("config") + .arg("shell") + .arg("install") + .arg("zsh") + .arg("--yes") + .current_dir(repo.root_path()); + + let output = cmd.output().unwrap(); + assert!( + output.status.success(), + "install should treat the existing manual line as configured:\nstderr:\n{}", + String::from_utf8_lossy(&output.stderr) + ); + + let content = fs::read_to_string(&zshrc_path).unwrap(); + let count = content.matches("wt config shell init").count(); + assert_eq!(count, 1, "Should not append a duplicate shell init line"); +} + #[rstest] fn test_configure_shell_fish(repo: TestRepo, temp_home: TempDir) { let settings = setup_home_snapshot_settings(&temp_home); @@ -363,7 +433,11 @@ fn test_configure_shell_fish_legacy_cleanup_even_when_already_exists( // Also create completions (so it reports "all already configured") let completions_d = temp_home.path().join(".config/fish/completions"); fs::create_dir_all(&completions_d).unwrap(); - fs::write(completions_d.join("wt.fish"), "# completions").unwrap(); + fs::write( + completions_d.join("wt.fish"), + "# worktrunk completions for fish\ncomplete --command wt\n", + ) + .unwrap(); // Create legacy conf.d file (old location that should be cleaned up) let conf_d = temp_home.path().join(".config/fish/conf.d"); @@ -429,7 +503,11 @@ fn test_uninstall_shell_fish_legacy_conf_d_cleanup(repo: TestRepo, temp_home: Te let completions_d = temp_home.path().join(".config/fish/completions"); fs::create_dir_all(&completions_d).unwrap(); let completions_file = completions_d.join("wt.fish"); - fs::write(&completions_file, "# completions").unwrap(); + fs::write( + &completions_file, + "# worktrunk completions for fish\ncomplete --command wt\n", + ) + .unwrap(); let settings = setup_home_snapshot_settings(&temp_home); settings.bind(|| { @@ -539,7 +617,11 @@ fn test_configure_shell_fish_dry_run_does_not_delete_legacy(repo: TestRepo, temp // Create completions (so it reports "all already configured") let completions_d = temp_home.path().join(".config/fish/completions"); fs::create_dir_all(&completions_d).unwrap(); - fs::write(completions_d.join("wt.fish"), "# completions").unwrap(); + fs::write( + completions_d.join("wt.fish"), + "# worktrunk completions for fish\ncomplete --command wt\n", + ) + .unwrap(); // Create legacy conf.d file that should NOT be deleted in dry-run mode let conf_d = temp_home.path().join(".config/fish/conf.d"); @@ -888,9 +970,9 @@ fn test_uninstall_shell(repo: TestRepo, temp_home: TempDir) { ----- stderr ----- ✓ Removed shell extension & completions for zsh @ ~/.zshrc ↳ No bash shell extension & completions in ~/.bashrc - ↳ No fish shell extension in ~/.config/fish/functions/wt.fish - ↳ No nu shell extension & completions in ~/.local/share/nushell/vendor/autoload/wt.nu - ↳ No fish completions in ~/.config/fish/completions/wt.fish + ↳ No fish shell extension in ~/.config/fish/functions + ↳ No nu shell extension & completions in ~/.local/share/nushell/vendor/autoload + ↳ No fish completions in ~/.config/fish/completions ✓ Removed integration from 1 shell ↳ Restart shell to complete uninstall @@ -952,9 +1034,9 @@ fn test_uninstall_shell_multiple(repo: TestRepo, temp_home: TempDir) { ----- stderr ----- ✓ Removed shell extension & completions for bash @ ~/.bashrc ✓ Removed shell extension & completions for zsh @ ~/.zshrc - ↳ No fish shell extension in ~/.config/fish/functions/wt.fish - ↳ No nu shell extension & completions in ~/.local/share/nushell/vendor/autoload/wt.nu - ↳ No fish completions in ~/.config/fish/completions/wt.fish + ↳ No fish shell extension in ~/.config/fish/functions + ↳ No nu shell extension & completions in ~/.local/share/nushell/vendor/autoload + ↳ No fish completions in ~/.config/fish/completions ✓ Removed integration from 2 shells ↳ Restart shell to complete uninstall @@ -1111,6 +1193,237 @@ fn test_install_uninstall_roundtrip(repo: TestRepo, temp_home: TempDir) { ); } +#[rstest] +fn test_uninstall_scan_removes_all_worktrunk_zsh_lines(repo: TestRepo, temp_home: TempDir) { + let zshrc_path = temp_home.path().join(".zshrc"); + fs::write( + &zshrc_path, + "# Existing config\nif command -v wt >/dev/null 2>&1; then eval \"$(command wt config shell init zsh)\"; fi\n", + ) + .unwrap(); + + let mut install_cmd = wt_command(); + repo.configure_wt_cmd(&mut install_cmd); + set_temp_home_env(&mut install_cmd, temp_home.path()); + install_cmd.env("SHELL", "/bin/zsh"); + install_cmd.env("WORKTRUNK_TEST_COMPINIT_CONFIGURED", "1"); + install_cmd + .args([ + "config", "shell", "install", "zsh", "--yes", "--cmd", "git-wt", + ]) + .current_dir(repo.root_path()); + + let install_output = install_cmd.output().expect("Failed to execute install"); + assert!( + install_output.status.success(), + "Install should succeed:\nstderr: {}", + String::from_utf8_lossy(&install_output.stderr) + ); + let installed = fs::read_to_string(&zshrc_path).unwrap(); + assert!(installed.contains("git-wt config shell init zsh")); + + // A hand-written line in the `git wt` dispatch form, which install never + // emits: uninstall recognizes worktrunk's integration by whatever name it + // is invoked under, not only the names install writes. + fs::write( + &zshrc_path, + format!("{installed}eval \"$(git wt config shell init zsh)\"\n"), + ) + .unwrap(); + + let mut uninstall_cmd = wt_command(); + repo.configure_wt_cmd(&mut uninstall_cmd); + set_temp_home_env(&mut uninstall_cmd, temp_home.path()); + uninstall_cmd.env("SHELL", "/bin/zsh"); + uninstall_cmd + .args(["config", "shell", "uninstall", "zsh", "--yes"]) + .current_dir(repo.root_path()); + + let uninstall_output = uninstall_cmd.output().expect("Failed to execute uninstall"); + assert!( + uninstall_output.status.success(), + "Uninstall should succeed:\nstderr: {}", + String::from_utf8_lossy(&uninstall_output.stderr) + ); + + let content = fs::read_to_string(&zshrc_path).unwrap(); + assert!( + !content.contains("git-wt config shell init zsh"), + "Custom command integration should be removed" + ); + assert!( + !content.contains("wt config shell init zsh"), + "Default command and `git wt` integration should also be removed (scan-all)" + ); + assert!( + content.contains("# Existing config"), + "Unrelated comment should be preserved" + ); +} + +#[rstest] +fn test_uninstall_scan_removes_custom_cmd_fish_files(repo: TestRepo, temp_home: TempDir) { + let fish_functions = temp_home.path().join(".config/fish/functions"); + let fish_completions = temp_home.path().join(".config/fish/completions"); + fs::create_dir_all(&fish_functions).unwrap(); + fs::create_dir_all(&fish_completions).unwrap(); + let default_function = fish_functions.join("wt.fish"); + let default_completion = fish_completions.join("wt.fish"); + fs::write(&default_function, "function wt\nend\n").unwrap(); + fs::write(&default_completion, "complete --command wt\n").unwrap(); + + // A second worktrunk-managed wrapper left over from an earlier install + // under another name, and an orphaned completion whose wrapper is already + // gone: both carry worktrunk's markers, so scan-all removes them too. + let stale_wrapper = fish_functions.join("old-wt.fish"); + fs::write( + &stale_wrapper, + "# worktrunk shell integration for fish\nfunction old-wt\nend\n", + ) + .unwrap(); + let orphan_completion = fish_completions.join("stale.fish"); + fs::write( + &orphan_completion, + "# worktrunk completions for fish\ncomplete --command stale\n", + ) + .unwrap(); + + let mut install_cmd = wt_command(); + repo.configure_wt_cmd(&mut install_cmd); + set_temp_home_env(&mut install_cmd, temp_home.path()); + install_cmd.env("SHELL", "/bin/fish"); + install_cmd + .args([ + "config", "shell", "install", "fish", "--yes", "--cmd", "git-wt", + ]) + .current_dir(repo.root_path()); + + let install_output = install_cmd.output().expect("Failed to execute install"); + assert!( + install_output.status.success(), + "Install should succeed:\nstderr: {}", + String::from_utf8_lossy(&install_output.stderr) + ); + let custom_function = fish_functions.join("git-wt.fish"); + let custom_completion = fish_completions.join("git-wt.fish"); + assert!(custom_function.exists()); + assert!(custom_completion.exists()); + + let mut uninstall_cmd = wt_command(); + repo.configure_wt_cmd(&mut uninstall_cmd); + set_temp_home_env(&mut uninstall_cmd, temp_home.path()); + uninstall_cmd.env("SHELL", "/bin/fish"); + uninstall_cmd + .args(["config", "shell", "uninstall", "fish", "--yes"]) + .current_dir(repo.root_path()); + + let uninstall_output = uninstall_cmd.output().expect("Failed to execute uninstall"); + assert!( + uninstall_output.status.success(), + "Uninstall should succeed:\nstderr: {}", + String::from_utf8_lossy(&uninstall_output.stderr) + ); + + // Scan-all removes every worktrunk-managed file (git-wt install, the stale + // second wrapper, the orphaned completion) but leaves the hand-written + // wt.fish stubs alone — they carry no worktrunk marker. + assert!(!custom_function.exists()); + assert!(!custom_completion.exists()); + assert!(!stale_wrapper.exists()); + assert!(!orphan_completion.exists()); + assert!(default_function.exists()); + assert!(default_completion.exists()); +} + +#[cfg(unix)] +#[rstest] +fn test_uninstall_shell_powershell_removes_integration_line(repo: TestRepo, temp_home: TempDir) { + // Unix-only: the Windows profile path comes from the real Documents + // folder, which a temp HOME does not redirect. + let profile_dir = temp_home.path().join(".config/powershell"); + fs::create_dir_all(&profile_dir).unwrap(); + let profile = profile_dir.join("Microsoft.PowerShell_profile.ps1"); + fs::write( + &profile, + "# my profile\nInvoke-Expression (& wt config shell init powershell)\nSet-Alias ll Get-ChildItem\n", + ) + .unwrap(); + + let mut cmd = wt_command(); + repo.configure_wt_cmd(&mut cmd); + set_temp_home_env(&mut cmd, temp_home.path()); + cmd.args(["config", "shell", "uninstall", "powershell", "--yes"]) + .current_dir(repo.root_path()); + + let output = cmd.output().expect("Failed to execute uninstall"); + assert!( + output.status.success(), + "Uninstall should succeed:\nstderr: {}", + String::from_utf8_lossy(&output.stderr) + ); + + // The old pre-Out-String integration line is removed; everything else stays. + let content = fs::read_to_string(&profile).unwrap(); + assert!( + !content.contains("config shell init"), + "Integration line should be removed:\n{content}" + ); + assert!(content.contains("# my profile")); + assert!(content.contains("Set-Alias ll Get-ChildItem")); +} + +#[rstest] +fn test_uninstall_scan_removes_custom_cmd_nushell_file(repo: TestRepo, temp_home: TempDir) { + let home = canonical_temp_home(&temp_home); + // Pin the vendor-autoload dir so the target is deterministic across + // platforms and independent of whether `nu` is on the runner's PATH. + let nu_autoload = home.join(".local/share/nushell/vendor/autoload"); + fs::create_dir_all(&nu_autoload).unwrap(); + let default_config = nu_autoload.join("wt.nu"); + fs::write(&default_config, "def --env --wrapped wt [] {}\n").unwrap(); + + let mut install_cmd = wt_command(); + repo.configure_wt_cmd(&mut install_cmd); + set_temp_home_env(&mut install_cmd, temp_home.path()); + install_cmd.env("WORKTRUNK_TEST_NU_VENDOR_AUTOLOAD_DIR", &nu_autoload); + install_cmd.env("SHELL", "/bin/nu"); + install_cmd + .args([ + "config", "shell", "install", "nu", "--yes", "--cmd", "git-wt", + ]) + .current_dir(repo.root_path()); + + let install_output = install_cmd.output().expect("Failed to execute install"); + assert!( + install_output.status.success(), + "Install should succeed:\nstderr: {}", + String::from_utf8_lossy(&install_output.stderr) + ); + let custom_config = nu_autoload.join("git-wt.nu"); + assert!(custom_config.exists()); + + let mut uninstall_cmd = wt_command(); + repo.configure_wt_cmd(&mut uninstall_cmd); + set_temp_home_env(&mut uninstall_cmd, temp_home.path()); + uninstall_cmd.env("WORKTRUNK_TEST_NU_VENDOR_AUTOLOAD_DIR", &nu_autoload); + uninstall_cmd.env("SHELL", "/bin/nu"); + uninstall_cmd + .args(["config", "shell", "uninstall", "nu", "--yes"]) + .current_dir(repo.root_path()); + + let uninstall_output = uninstall_cmd.output().expect("Failed to execute uninstall"); + assert!( + uninstall_output.status.success(), + "Uninstall should succeed:\nstderr: {}", + String::from_utf8_lossy(&uninstall_output.stderr) + ); + + // Scan-all removes git-wt.nu (worktrunk-managed by content) but leaves the + // hand-written wt.nu stub alone (no `config shell init … | source` marker). + assert!(!custom_config.exists()); + assert!(default_config.exists()); +} + #[rstest] fn test_install_uninstall_no_blank_line_accumulation(repo: TestRepo, temp_home: TempDir) { // Create initial config file matching the user's real zshrc structure @@ -1556,7 +1869,11 @@ fn test_uninstall_shell_dry_run_fish(repo: TestRepo, temp_home: TempDir) { let completions_d = temp_home.path().join(".config/fish/completions"); fs::create_dir_all(&completions_d).unwrap(); let completions_file = completions_d.join("wt.fish"); - fs::write(&completions_file, "# fish completions").unwrap(); + fs::write( + &completions_file, + "# worktrunk completions for fish\ncomplete --command wt\n", + ) + .unwrap(); let settings = setup_home_snapshot_settings(&temp_home); settings.bind(|| { @@ -1599,7 +1916,11 @@ fn test_uninstall_shell_dry_run_fish_canonical(repo: TestRepo, temp_home: TempDi let completions_d = temp_home.path().join(".config/fish/completions"); fs::create_dir_all(&completions_d).unwrap(); let completions_file = completions_d.join("wt.fish"); - fs::write(&completions_file, "# fish completions").unwrap(); + fs::write( + &completions_file, + "# worktrunk completions for fish\ncomplete --command wt\n", + ) + .unwrap(); let settings = setup_home_snapshot_settings(&temp_home); settings.bind(|| { diff --git a/tests/integration_tests/init.rs b/tests/integration_tests/init.rs index 5c6d408955..c31157ed91 100644 --- a/tests/integration_tests/init.rs +++ b/tests/integration_tests/init.rs @@ -5,10 +5,11 @@ //! are not the primary shell integration path on Windows (PowerShell is). #![cfg(not(windows))] -use crate::common::{TestRepo, add_standard_env_redactions, repo, wt_command}; +use crate::common::{TestRepo, add_standard_env_redactions, repo, wt_bin, wt_command}; use insta::Settings; use insta_cmd::assert_cmd_snapshot; use rstest::rstest; +use std::process::Command; /// Helper to create snapshot for config shell init command fn snapshot_init(test_name: &str, repo: &TestRepo, shell: &str, extra_args: &[&str]) { @@ -69,3 +70,64 @@ fn test_init_invalid_shell(repo: TestRepo) { "); }); } + +#[rstest] +#[case("bash")] +#[case("fish")] +#[case("nu")] +#[case("powershell")] +#[case("zsh")] +fn test_init_rejects_unsafe_cmd(#[case] shell: &str, repo: TestRepo) { + let mut cmd = wt_command(); + repo.configure_wt_cmd(&mut cmd); + cmd.arg("config") + .arg("shell") + .arg("init") + .arg(shell) + .arg("--cmd") + .arg("wt; touch /tmp/pwn") + .current_dir(repo.root_path()); + + let output = cmd.output().unwrap(); + + assert!(!output.status.success()); + assert!( + output.stdout.is_empty(), + "unsafe command name must not emit shell code:\n{}", + String::from_utf8_lossy(&output.stdout) + ); + assert!( + String::from_utf8_lossy(&output.stderr).contains("Invalid shell integration command name"), + "expected validation error, got:\n{}", + String::from_utf8_lossy(&output.stderr) + ); +} + +#[rstest] +fn test_init_rejects_unsafe_argv0_command_name(repo: TestRepo) { + let temp_dir = tempfile::tempdir().unwrap(); + let bad_bin = temp_dir.path().join("wt;touch"); + std::os::unix::fs::symlink(wt_bin(), &bad_bin).unwrap(); + + let mut cmd = Command::new(&bad_bin); + repo.configure_wt_cmd(&mut cmd); + cmd.arg("config") + .arg("shell") + .arg("init") + .arg("bash") + .current_dir(repo.root_path()); + + let output = cmd.output().unwrap(); + + assert!(!output.status.success()); + assert!( + output.stdout.is_empty(), + "unsafe argv[0] command name must not emit shell code:\n{}", + String::from_utf8_lossy(&output.stdout) + ); + assert!( + String::from_utf8_lossy(&output.stderr).contains("Invalid shell integration command name"), + "expected validation error, got:\n{}", + String::from_utf8_lossy(&output.stderr) + ); +}