diff --git a/CHANGELOG.md b/CHANGELOG.md index 4432e00b9..dc0814a87 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - Fixed shell hook spawning a nested `devenv shell` when `devenv shell` was entered manually. Follow-up to [#2815](https://github.com/cachix/devenv/pull/2815). - Fixed "zoxide: infinite loop detected" when using `zoxide init --cmd=cd fish` and `cd`-ing into a devenv project. The fish hook now defers spawning `devenv shell` to the next prompt instead of spawning inline inside the PWD event handler, so in-progress shell state never leaks into the devenv shell ([#2841](https://github.com/cachix/devenv/issues/2841)). - Fixed stale task and option results that lingered when `devenv.nix` was edited while a command was already evaluating. The eval cache no longer stores results whose tracked input files were modified mid-evaluation, so the next run no longer needs `.devenv/nix-eval-cache.db*` to be wiped to see the new definitions ([#2745](https://github.com/cachix/devenv/issues/2745)). +- Fixed shell hook auto-activation reusing a stale `.devenv/exit-dir` when a shell override blocked cleanup (for example a custom `rm` function), which could send the next shell exit to an old directory. ### Improvements diff --git a/devenv/hooks/hook.fish b/devenv/hooks/hook.fish index 549f1b37a..506cf65f6 100644 --- a/devenv/hooks/hook.fish +++ b/devenv/hooks/hook.fish @@ -48,12 +48,14 @@ end # Spawn devenv shell in $project_dir and follow the user if they cd'd out. function _devenv_hook_activate set -l project_dir $argv[1] + set -l exit_dir_file "$project_dir/.devenv/exit-dir" + # Drop stale state from an earlier failed cleanup before launching a new shell. + command rm -f "$exit_dir_file" env -C $project_dir _DEVENV_HOOK_DIR=$project_dir devenv shell # If the devenv shell exited due to cd outside the project, follow the user there - set -l exit_dir_file "$project_dir/.devenv/exit-dir" if test -f "$exit_dir_file" set -l target_dir (cat "$exit_dir_file") - rm -f "$exit_dir_file" + command rm -f "$exit_dir_file" if test -d "$target_dir" builtin cd "$target_dir" end diff --git a/devenv/hooks/hook.nu b/devenv/hooks/hook.nu index e4458d616..a815f5319 100644 --- a/devenv/hooks/hook.nu +++ b/devenv/hooks/hook.nu @@ -27,13 +27,15 @@ $env.config = ($env.config | upsert hooks.env_change.PWD ( if $result.exit_code == 0 { let dir = ($result.stdout | str trim) if $dir != "" { + let exit_dir_file = ($dir + "/.devenv/exit-dir") + # Drop stale state from an earlier failed cleanup before launching a new shell. + ^rm -f $exit_dir_file do { cd $dir; ^devenv shell } $env._DEVENV_HOOK_UNTRUSTED = "" # If the devenv shell exited due to cd outside the project, follow the user there - let exit_dir_file = ($dir + "/.devenv/exit-dir") if ($exit_dir_file | path exists) { let target_dir = (open $exit_dir_file | str trim) - rm -f $exit_dir_file + ^rm -f $exit_dir_file if ($target_dir | path exists) { cd $target_dir } @@ -63,13 +65,15 @@ $env.config = ($env.config | upsert hooks.pre_prompt ( if $result.exit_code == 0 { let dir = ($result.stdout | str trim) if $dir != "" { + let exit_dir_file = ($dir + "/.devenv/exit-dir") + # Drop stale state from an earlier failed cleanup before launching a new shell. + ^rm -f $exit_dir_file do { cd $dir; ^devenv shell } $env._DEVENV_HOOK_UNTRUSTED = "" # If the devenv shell exited due to cd outside the project, follow the user there - let exit_dir_file = ($dir + "/.devenv/exit-dir") if ($exit_dir_file | path exists) { let target_dir = (open $exit_dir_file | str trim) - rm -f $exit_dir_file + ^rm -f $exit_dir_file if ($target_dir | path exists) { cd $target_dir } diff --git a/devenv/hooks/hook.posix.sh b/devenv/hooks/hook.posix.sh index c96a7df55..3993d1c8d 100644 --- a/devenv/hooks/hook.posix.sh +++ b/devenv/hooks/hook.posix.sh @@ -38,12 +38,14 @@ _devenv_hook() { # Cache PWD before launching so a SIGINT/failure inside devenv shell # doesn't leave us re-launching on every prompt redraw. _DEVENV_HOOK_PWD="$PWD" - (cd "$project_dir" && _DEVENV_HOOK_DIR="$project_dir" devenv shell) local exit_dir_file="$project_dir/.devenv/exit-dir" + # Drop stale state from an earlier failed cleanup before launching a new shell. + command rm -f "$exit_dir_file" + (cd "$project_dir" && _DEVENV_HOOK_DIR="$project_dir" devenv shell) if [[ -f "$exit_dir_file" ]]; then local target_dir target_dir=$(cat "$exit_dir_file") - rm -f "$exit_dir_file" + command rm -f "$exit_dir_file" if [[ -d "$target_dir" ]]; then cd "$target_dir" _DEVENV_HOOK_PWD="$PWD" diff --git a/devenv/tests/hook_outer_shell_survives.rs b/devenv/tests/hook_outer_shell_survives.rs index c21af5329..17b4fe029 100644 --- a/devenv/tests/hook_outer_shell_survives.rs +++ b/devenv/tests/hook_outer_shell_survives.rs @@ -19,17 +19,30 @@ fn devenv_bin() -> &'static str { env!("CARGO_BIN_EXE_devenv") } +type PathOverrideFn = fn(&Path) -> String; +type ShellSnippetFn = fn() -> &'static str; + /// Available shells with the syntax needed to (a) source the hook from the /// real `devenv` binary and (b) override `PATH`. -fn shells() -> Vec<(&'static str, String, fn(&Path) -> String)> { +fn shells() -> Vec<(&'static str, String, PathOverrideFn, ShellSnippetFn)> { let bash_src = format!(r#"eval "$({} hook bash)""#, devenv_bin()); let fish_src = format!("{} hook fish | source", devenv_bin()); [ - ("bash", bash_src, bash_path_override as fn(&Path) -> String), - ("fish", fish_src, fish_path_override as fn(&Path) -> String), + ( + "bash", + bash_src, + bash_path_override as PathOverrideFn, + bash_block_rm as ShellSnippetFn, + ), + ( + "fish", + fish_src, + fish_path_override as PathOverrideFn, + fish_block_rm as ShellSnippetFn, + ), ] .into_iter() - .filter(|(s, _, _)| have(s)) + .filter(|(s, _, _, _)| have(s)) .collect() } @@ -41,6 +54,14 @@ fn fish_path_override(dir: &Path) -> String { format!("set -gx PATH {:?} $PATH", dir) } +fn bash_block_rm() -> &'static str { + "rm() { echo BLOCKED >&2; }" +} + +fn fish_block_rm() -> &'static str { + "function rm; echo BLOCKED >&2; end" +} + fn have(shell: &str) -> bool { Command::new("sh") .args(["-c", &format!("command -v {shell}")]) @@ -56,12 +77,20 @@ fn fake_project() -> tempfile::TempDir { } fn run(shell: &str, script: &str) -> std::process::Output { - Command::new(shell).arg("-c").arg(script).output().unwrap() + Command::new(shell) + .env_remove("DEVENV_ROOT") + .env_remove("_DEVENV_HOOK_DIR") + .env_remove("_DEVENV_HOOK_UNTRUSTED") + .env_remove("_DEVENV_ACTIVATE_DIR") + .arg("-c") + .arg(script) + .output() + .unwrap() } #[test] fn outer_shell_survives_cd_out() { - for (shell, src, _) in shells() { + for (shell, src, _, _) in shells() { let tmp = fake_project(); let script = format!( "export DEVENV_ROOT={root:?}\n{src}\ncd /\n_devenv_hook\necho SURVIVED\n", @@ -79,7 +108,7 @@ fn outer_shell_survives_cd_out() { #[test] fn inner_shell_exits_on_cd_out() { - for (shell, src, _) in shells() { + for (shell, src, _, _) in shells() { let tmp = fake_project(); let script = format!( "export DEVENV_ROOT={root:?}\nexport _DEVENV_HOOK_DIR={root:?}\n\ @@ -99,7 +128,7 @@ fn inner_shell_exits_on_cd_out() { #[test] fn no_respawn_inside_devenv_shell() { - for (shell, src, path_override) in shells() { + for (shell, src, path_override, _) in shells() { let tmp = fake_project(); // Shim `devenv` on PATH so any `hook-should-activate` / `shell` // invocation from inside the hook is recorded. @@ -133,3 +162,53 @@ fn no_respawn_inside_devenv_shell() { ); } } + +#[test] +fn stale_exit_dir_cleanup_bypasses_shell_rm_overrides() { + for (shell, src, path_override, block_rm) in shells() { + let tmp = fake_project(); + let exit_dir = tmp.path().join(".devenv/exit-dir"); + + // Shim `devenv` on PATH so the hook activates the temp project and + // exits the child shell immediately without writing a new exit-dir. + let bin_dir = tempfile::tempdir().unwrap(); + let fake = bin_dir.path().join("devenv"); + fs::write( + &fake, + format!( + "#!/usr/bin/env bash\ncase \"$1\" in\n hook-should-activate) printf '%s\\n' {:?} ;;\n shell) exit 0 ;;\n *) exit 1 ;;\nesac\n", + tmp.path(), + ), + ) + .unwrap(); + fs::set_permissions(&fake, fs::Permissions::from_mode(0o755)).unwrap(); + + let activate = if shell == "fish" { + format!("_devenv_hook_activate {:?}", tmp.path()) + } else { + "_devenv_hook".to_string() + }; + + let script = format!( + "cd {root:?}\nprintf '%s' / > {exit_dir:?}\n{src}\n{po}\n{block_rm}\n{activate}\npwd\n", + root = tmp.path(), + exit_dir = exit_dir, + po = path_override(bin_dir.path()), + block_rm = block_rm(), + activate = activate, + ); + let out = run(shell, &script); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!( + stdout + .lines() + .any(|line| line == tmp.path().to_string_lossy()), + "[{shell}] stale exit-dir changed directories even though rm was overridden.\nstdout: {stdout}\nstderr: {}", + String::from_utf8_lossy(&out.stderr), + ); + assert!( + !exit_dir.exists(), + "[{shell}] stale exit-dir was not removed when rm was overridden", + ); + } +}