From c49abd20fc07630157ff1fbcf068ddf07787e5d4 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Fri, 31 Jul 2026 16:48:50 +0800 Subject: [PATCH] fix(core): read the login shell from passwd, not the stale $SHELL $SHELL is a snapshot the session inherits at login, so chsh never moves it -- a GUI launch keeps reporting the shell that was current when the user logged in, and goes on doing so until they log out. The window's shell menu marked the wrong entry "default" for that whole stretch. Read the passwd entry instead, via getpwuid_r -- the reentrant form, since getpwuid returns a pointer into a shared static another thread's lookup can overwrite. $SHELL stays as the fallback for the rare case where the lookup fails. The three callers that each reached for the variable on their own -- the default-name lookup, the PATH enrichment that runs the login shell at startup, and the shell-integration kind probe -- now share the one function. Same commit fixes who wins a name in the menu. Candidates were login shell, then /etc/shells, then $PATH, and dedupe keeps the first -- so on a machine with a Homebrew bash, /etc/shells listing /bin/bash first handed the entry to macOS's 3.2 from 2007, old enough that bash-completion 2.x will not load against it. Probe $PATH before /etc/shells and widen the probe list to the POSIX shells, so the menu's "bash" is the binary typing bash would reach; /etc/shells still catches anything installed off $PATH. --- crates/tty7-core/src/core/shells.rs | 197 +++++++++++++++--- .../tty7-core/src/daemon/shell_integration.rs | 2 +- src/main.rs | 2 +- 3 files changed, 174 insertions(+), 27 deletions(-) diff --git a/crates/tty7-core/src/core/shells.rs b/crates/tty7-core/src/core/shells.rs index f91ab190..dca7c4df 100644 --- a/crates/tty7-core/src/core/shells.rs +++ b/crates/tty7-core/src/core/shells.rs @@ -49,20 +49,76 @@ pub fn detect_shells() -> Vec { pub fn default_shell_name(configured: Option<&str>) -> String { let program = match configured { Some(p) if !p.trim().is_empty() => p.to_string(), - _ => { - #[cfg(unix)] - { - std::env::var("SHELL").unwrap_or_else(|_| "sh".into()) - } - #[cfg(windows)] - { - windows_default_shell().to_string() - } - } + _ => login_shell(), }; basename(&program) } +/// The user's login shell, straight from the passwd database. +/// +/// `$SHELL` is a snapshot taken when the session logged in, so `chsh` does not +/// move it — a GUI launch inherits whatever was current at login and keeps +/// reporting it until the user logs out. passwd is the live value; `$SHELL` is +/// only the fallback for the rare setup where the lookup fails (a directory +/// service that is down, a uid with no passwd entry). +pub fn login_shell() -> String { + #[cfg(unix)] + { + pick_login_shell(passwd_shell(), std::env::var("SHELL").ok()) + } + #[cfg(windows)] + { + windows_default_shell().to_string() + } +} + +#[cfg_attr(windows, allow(dead_code))] +fn pick_login_shell(passwd: Option, env: Option) -> String { + passwd + .into_iter() + .chain(env) + .map(|s| s.trim().to_string()) + .find(|s| !s.is_empty()) + .unwrap_or_else(|| "sh".into()) +} + +/// `getpwuid_r` — the reentrant form, because `getpwuid` hands back a pointer +/// into a shared static that another thread's lookup can overwrite under us. +#[cfg(unix)] +fn passwd_shell() -> Option { + let mut buf_len = match unsafe { libc::sysconf(libc::_SC_GETPW_R_SIZE_MAX) } { + n if n > 0 => n as usize, + _ => 1024, + }; + loop { + let mut pwd: libc::passwd = unsafe { std::mem::zeroed() }; + let mut buf = vec![0 as libc::c_char; buf_len]; + let mut found: *mut libc::passwd = std::ptr::null_mut(); + let rc = unsafe { + libc::getpwuid_r( + libc::getuid(), + &mut pwd, + buf.as_mut_ptr(), + buf.len(), + &mut found, + ) + }; + // ERANGE just means the buffer was too small; anything else is fatal. + if rc == libc::ERANGE && buf_len < 64 * 1024 { + buf_len *= 2; + continue; + } + if rc != 0 || found.is_null() || pwd.pw_shell.is_null() { + return None; + } + let shell = unsafe { std::ffi::CStr::from_ptr(pwd.pw_shell) } + .to_str() + .ok()? + .to_string(); + return Some(shell); + } +} + fn basename(program: &str) -> String { let base = Path::new(program) .file_name() @@ -105,8 +161,17 @@ fn unix_shells_from( out } +/// Shell names worth probing along `$PATH`. +/// +/// The POSIX-y ones are here as well as the newer shells: `/etc/shells` lists +/// only what the system ships, so on a box with a Homebrew `bash` the entry +/// that wins the name is `/bin/bash` — macOS's 3.2 from 2007, which is old +/// enough that bash-completion 2.x refuses to load. Probing `$PATH` first makes +/// the menu's "bash" the same binary typing `bash` would reach. #[cfg_attr(windows, allow(dead_code))] -const PATH_PROBED_SHELLS: [&str; 5] = ["fish", "nu", "pwsh", "elvish", "xonsh"]; +const PATH_PROBED_SHELLS: [&str; 12] = [ + "bash", "zsh", "fish", "nu", "pwsh", "elvish", "xonsh", "sh", "ksh", "dash", "tcsh", "csh", +]; #[cfg_attr(windows, allow(dead_code))] fn path_shell_candidates(path_var: &str) -> Vec { @@ -122,13 +187,14 @@ fn path_shell_candidates(path_var: &str) -> Vec { #[cfg(unix)] fn detect_unix() -> Vec { - let login = std::env::var("SHELL").ok().filter(|s| !s.is_empty()); let etc = std::fs::read_to_string("/etc/shells").unwrap_or_default(); let path_var = std::env::var("PATH").unwrap_or_default(); - let candidates = login - .into_iter() - .chain(parse_etc_shells(&etc)) - .chain(path_shell_candidates(&path_var)); + // Order decides who wins a name, since dedupe keeps the first: the login + // shell must be reachable, then whatever `$PATH` resolves each name to, + // and `/etc/shells` last to catch shells installed outside `$PATH`. + let candidates = std::iter::once(login_shell()) + .chain(path_shell_candidates(&path_var)) + .chain(parse_etc_shells(&etc)); unix_shells_from(candidates, |p| Path::new(p).is_file()) } @@ -346,36 +412,117 @@ mod tests { #[test] fn path_shell_candidates_expand_dirs_in_order_skipping_relative() { let cands = path_shell_candidates("/opt/homebrew/bin:relative:.:/usr/bin/:"); - assert_eq!(cands[0], "/opt/homebrew/bin/fish"); - assert_eq!(cands[1], "/usr/bin/fish"); + // Each name walks $PATH in order, so the earlier dir gets first refusal. + let first = PATH_PROBED_SHELLS[0]; + assert_eq!(cands[0], format!("/opt/homebrew/bin/{first}")); + assert_eq!(cands[1], format!("/usr/bin/{first}")); assert!(cands.contains(&"/opt/homebrew/bin/nu".to_string())); assert!(cands.iter().all(|c| c.starts_with('/'))); assert_eq!(cands.len(), PATH_PROBED_SHELLS.len() * 2); } #[test] - fn unregistered_path_shells_are_detected_after_etc_shells() { - let etc = ["/bin/zsh".to_string(), "/bin/bash".to_string()]; - let candidates = etc + fn etc_shells_still_contributes_what_path_does_not_reach() { + let etc = ["/bin/zsh".to_string(), "/opt/weird/ksh".to_string()]; + let candidates = path_shell_candidates("/opt/homebrew/bin:/usr/bin") .into_iter() - .chain(path_shell_candidates("/opt/homebrew/bin:/usr/bin")); + .chain(etc); let exists = |p: &str| { matches!( p, - "/bin/zsh" | "/bin/bash" | "/opt/homebrew/bin/fish" | "/usr/bin/zsh" + "/bin/zsh" | "/usr/bin/zsh" | "/opt/homebrew/bin/fish" | "/opt/weird/ksh" ) }; let got = unix_shells_from(candidates, exists); assert_eq!( got, vec![ - DetectedShell::bare("zsh", "/bin/zsh"), - DetectedShell::bare("bash", "/bin/bash"), + // $PATH resolves zsh to /usr/bin/zsh, so /bin/zsh loses the name + DetectedShell::bare("zsh", "/usr/bin/zsh"), DetectedShell::bare("fish", "/opt/homebrew/bin/fish"), + // never on $PATH, so only /etc/shells knows about it + DetectedShell::bare("ksh", "/opt/weird/ksh"), ] ); } + /// The bug: `/etc/shells` lists `/bin/bash` (macOS 3.2) before a Homebrew + /// `bash`, so dedupe-by-name handed the menu entry to the 2007 build. + #[test] + fn a_path_shell_beats_the_same_name_in_etc_shells() { + let etc = [ + "/bin/bash".to_string(), + "/opt/homebrew/bin/bash".to_string(), + ]; + let candidates = path_shell_candidates("/opt/homebrew/bin:/usr/bin") + .into_iter() + .chain(etc.iter().cloned()); + let exists = |p: &str| matches!(p, "/bin/bash" | "/opt/homebrew/bin/bash"); + let got = unix_shells_from(candidates, exists); + assert_eq!( + got, + vec![DetectedShell::bare("bash", "/opt/homebrew/bin/bash")] + ); + + // …and with the old ordering the stale one would have won. + let old_order = etc + .into_iter() + .chain(path_shell_candidates("/opt/homebrew/bin")); + assert_eq!( + unix_shells_from(old_order, exists), + vec![DetectedShell::bare("bash", "/bin/bash")] + ); + } + + #[test] + fn the_login_shell_outranks_path_for_its_own_name() { + // A login shell that $PATH would otherwise resolve elsewhere still has + // to be the entry the menu offers. + let candidates = + std::iter::once("/opt/custom/bin/zsh".to_string()).chain(path_shell_candidates("/bin")); + let exists = |p: &str| matches!(p, "/opt/custom/bin/zsh" | "/bin/zsh" | "/bin/bash"); + let got = unix_shells_from(candidates, exists); + assert_eq!(got[0], DetectedShell::bare("zsh", "/opt/custom/bin/zsh")); + assert!(!got.iter().any(|s| s.program == "/bin/zsh")); + } + + /// passwd is the live value; `$SHELL` is a login-time snapshot that `chsh` + /// cannot move, so it must never win. + #[test] + fn login_shell_prefers_passwd_over_a_stale_env() { + assert_eq!( + pick_login_shell( + Some("/opt/homebrew/bin/bash".into()), + Some("/bin/zsh".into()) + ), + "/opt/homebrew/bin/bash" + ); + // passwd unreadable, or the entry is blank — fall back to $SHELL + assert_eq!(pick_login_shell(None, Some("/bin/zsh".into())), "/bin/zsh"); + assert_eq!( + pick_login_shell(Some(" ".into()), Some("/bin/zsh".into())), + "/bin/zsh" + ); + // neither available + assert_eq!(pick_login_shell(None, None), "sh"); + assert_eq!( + pick_login_shell(Some(String::new()), Some(String::new())), + "sh" + ); + } + + #[cfg(unix)] + #[test] + fn passwd_shell_reads_this_users_entry() { + // Every account running the suite has a shell in passwd; the point is + // that the lookup works and returns an absolute path, not a specific one. + let got = passwd_shell().expect("passwd lookup should succeed"); + assert!( + got.starts_with('/'), + "expected an absolute path, got {got:?}" + ); + } + #[test] fn parse_wsl_list_decodes_utf16le_and_filters() { let text = "Ubuntu\r\ndocker-desktop\r\ndocker-desktop-data\r\nDebian\r\n\r\n"; diff --git a/crates/tty7-core/src/daemon/shell_integration.rs b/crates/tty7-core/src/daemon/shell_integration.rs index 6cae6d2f..49b4aaa2 100644 --- a/crates/tty7-core/src/daemon/shell_integration.rs +++ b/crates/tty7-core/src/daemon/shell_integration.rs @@ -594,7 +594,7 @@ enum ShellKind { fn shell_kind(program: Option<&str>) -> Option { let owned = match program { Some(p) => p.to_string(), - None => std::env::var("SHELL").ok()?, + None => crate::core::shells::login_shell(), }; let base = Path::new(&owned) .file_name()? diff --git a/src/main.rs b/src/main.rs index 88967183..4a7c3869 100644 --- a/src/main.rs +++ b/src/main.rs @@ -123,7 +123,7 @@ fn merge_paths(primary: &str, secondary: &str) -> String { #[cfg(unix)] fn enrich_path_from_login_shell() { - let shell = std::env::var("SHELL").unwrap_or_else(|_| "/bin/sh".into()); + let shell = crate::core::shells::login_shell(); let cmd = if std::path::Path::new(&shell).file_name() == Some("fish".as_ref()) { "string join ':' $PATH" } else {