Skip to content

Commit 13ee395

Browse files
authored
fix(tui): harden external keybindings opener
Close the remaining Windows opener hardening gaps. - pass paths to cmd.exe start as explicit quoted arguments - prefer absolute SystemRoot, then absolute WINDIR, then C:\Windows - add regression coverage for quoting and system-root fallback behavior
1 parent 37d887e commit 13ee395

1 file changed

Lines changed: 85 additions & 7 deletions

File tree

src-rust/crates/tui/src/app.rs

Lines changed: 85 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6572,11 +6572,11 @@ impl App {
65726572
}
65736573

65746574
// Helper function to open a file in the user's external editor
6575-
fn open_file_externally(path: &std::path::Path) -> Result<(), Box<dyn std::error::Error>> {
6575+
fn open_file_externally(path: &std::path::Path) -> std::io::Result<()> {
65766576
// Try to open with the system's default application
65776577
#[cfg(target_os = "macos")]
65786578
{
6579-
std::process::Command::new("open")
6579+
std::process::Command::new("/usr/bin/open")
65806580
.arg(path)
65816581
.spawn()?;
65826582
Ok(())
@@ -6592,10 +6592,20 @@ fn open_file_externally(path: &std::path::Path) -> Result<(), Box<dyn std::error
65926592

65936593
#[cfg(target_os = "windows")]
65946594
{
6595-
std::process::Command::new("cmd")
6596-
.args(&["/C", "start", ""])
6597-
.arg(path)
6598-
.spawn()?;
6595+
let system_root = windows_system_root();
6596+
let mut command = std::process::Command::new(
6597+
std::path::Path::new(&system_root)
6598+
.join("System32")
6599+
.join("cmd.exe"),
6600+
);
6601+
let quoted_path = quote_windows_cmd_path(path);
6602+
command
6603+
.args(["/C", "start", ""])
6604+
.arg(quoted_path)
6605+
.env_clear()
6606+
.env("SystemRoot", &system_root)
6607+
.env("WINDIR", &system_root);
6608+
command.spawn()?;
65996609
Ok(())
66006610
}
66016611

@@ -6611,10 +6621,38 @@ fn open_file_externally(path: &std::path::Path) -> Result<(), Box<dyn std::error
66116621
Err(_) => continue,
66126622
}
66136623
}
6614-
Err("No suitable editor found".into())
6624+
Err(std::io::Error::new(
6625+
std::io::ErrorKind::NotFound,
6626+
"No suitable editor found",
6627+
))
66156628
}
66166629
}
66176630

6631+
#[cfg(any(test, target_os = "windows"))]
6632+
fn quote_windows_cmd_path(path: &std::path::Path) -> std::ffi::OsString {
6633+
let mut quoted = std::ffi::OsString::from("\"");
6634+
quoted.push(path.as_os_str());
6635+
quoted.push("\"");
6636+
quoted
6637+
}
6638+
6639+
#[cfg(any(test, target_os = "windows"))]
6640+
fn windows_system_root_from_env(
6641+
system_root: Option<std::ffi::OsString>,
6642+
windir: Option<std::ffi::OsString>,
6643+
) -> std::ffi::OsString {
6644+
system_root
6645+
.into_iter()
6646+
.chain(windir)
6647+
.find(|root| std::path::Path::new(root).is_absolute())
6648+
.unwrap_or_else(|| std::ffi::OsString::from(r"C:\Windows"))
6649+
}
6650+
6651+
#[cfg(target_os = "windows")]
6652+
fn windows_system_root() -> std::ffi::OsString {
6653+
windows_system_root_from_env(std::env::var_os("SystemRoot"), std::env::var_os("WINDIR"))
6654+
}
6655+
66186656
#[cfg(test)]
66196657
mod tests {
66206658
use super::*;
@@ -6635,6 +6673,46 @@ mod tests {
66356673
}
66366674
}
66376675

6676+
#[test]
6677+
fn windows_cmd_path_is_passed_as_quoted_argument() {
6678+
let path = std::path::Path::new(r"C:\temp\safe & literal.txt");
6679+
6680+
assert_eq!(
6681+
quote_windows_cmd_path(path).to_string_lossy(),
6682+
r#""C:\temp\safe & literal.txt""#
6683+
);
6684+
}
6685+
6686+
#[test]
6687+
fn windows_system_root_prefers_absolute_system_root() {
6688+
let root = windows_system_root_from_env(
6689+
Some(std::ffi::OsString::from("/windows")),
6690+
Some(std::ffi::OsString::from("/windir")),
6691+
);
6692+
6693+
assert_eq!(root, std::ffi::OsString::from("/windows"));
6694+
}
6695+
6696+
#[test]
6697+
fn windows_system_root_falls_back_to_absolute_windir() {
6698+
let root = windows_system_root_from_env(
6699+
Some(std::ffi::OsString::from("relative-system-root")),
6700+
Some(std::ffi::OsString::from("/windir")),
6701+
);
6702+
6703+
assert_eq!(root, std::ffi::OsString::from("/windir"));
6704+
}
6705+
6706+
#[test]
6707+
fn windows_system_root_ignores_relative_paths() {
6708+
let root = windows_system_root_from_env(
6709+
Some(std::ffi::OsString::from("relative-system-root")),
6710+
Some(std::ffi::OsString::from("relative-windir")),
6711+
);
6712+
6713+
assert_eq!(root, std::ffi::OsString::from(r"C:\Windows"));
6714+
}
6715+
66386716
// ---- signal_bg_task_completion tests ----
66396717

66406718
fn make_bg_completion(success: bool) -> claurst_tools::BgTaskCompletion {

0 commit comments

Comments
 (0)