diff --git a/cli/src/commands/serve_web.rs b/cli/src/commands/serve_web.rs index 7f200d4341f..959763a431d 100644 --- a/cli/src/commands/serve_web.rs +++ b/cli/src/commands/serve_web.rs @@ -15,7 +15,6 @@ use hyper::service::{make_service_fn, service_fn}; use hyper::{Body, Request, Response, Server}; use tokio::io::{AsyncBufReadExt, BufReader}; use tokio::pin; -use tokio::process::Command; use crate::async_pipe::{ get_socket_name, get_socket_rw_stream, listen_socket_rw_stream, AsyncPipe, @@ -29,6 +28,7 @@ use crate::tunnels::shutdown_signal::ShutdownRequest; use crate::update_service::{ unzip_downloaded_release, Platform, Release, TargetKind, UpdateService, }; +use crate::util::command::new_script_command; use crate::util::errors::AnyError; use crate::util::http::{self, ReqwestSimpleHttp}; use crate::util::io::SilentCopyProgress; @@ -679,17 +679,7 @@ impl ConnectionManager { let socket_path = get_socket_name(); - #[cfg(not(windows))] - let mut cmd = Command::new(&executable); - #[cfg(windows)] - let mut cmd = { - let mut cmd = Command::new("cmd"); - cmd.arg("/Q"); - cmd.arg("/C"); - cmd.arg(&executable); - cmd - }; - + let mut cmd = new_script_command(&executable); cmd.stdin(std::process::Stdio::null()); cmd.stderr(std::process::Stdio::piped()); cmd.stdout(std::process::Stdio::piped()); diff --git a/cli/src/commands/tunnels.rs b/cli/src/commands/tunnels.rs index c0b3f3f01f9..8c4270ad453 100644 --- a/cli/src/commands/tunnels.rs +++ b/cli/src/commands/tunnels.rs @@ -51,6 +51,7 @@ use crate::{ }, util::{ app_lock::AppMutex, + command::new_std_command, errors::{wrap, AnyError, CodeError}, prereqs::PreReqChecker, }, @@ -604,7 +605,7 @@ async fn serve_with_csa( // reuse current args, but specify no-forward since tunnels will // already be running in this process, and we cannot do a login let args = std::env::args().skip(1).collect::>(); - let exit = std::process::Command::new(current_exe) + let exit = new_std_command(current_exe) .args(args) .spawn() .map_err(|e| wrap(e, "error respawning after update"))? diff --git a/cli/src/desktop/version_manager.rs b/cli/src/desktop/version_manager.rs index bb0f1011ab6..f2a093b5c77 100644 --- a/cli/src/desktop/version_manager.rs +++ b/cli/src/desktop/version_manager.rs @@ -18,7 +18,10 @@ use crate::{ log, state::{LauncherPaths, PersistedState}, update_service::Platform, - util::errors::{AnyError, InvalidRequestedVersion}, + util::{ + command::new_std_command, + errors::{AnyError, InvalidRequestedVersion}, + }, }; /// Parsed instance that a user can request. @@ -110,7 +113,7 @@ impl CodeVersionManager { // Check whether the user is supplying a path to the CLI directly (e.g. #164622) if let Ok(true) = path.metadata().map(|m| m.is_file()) { - let result = std::process::Command::new(path) + let result = new_std_command(path) .args(["--version"]) .output() .map(|o| o.status.success()); @@ -270,7 +273,7 @@ fn detect_installed_program(log: &log::Logger) -> io::Result> { // the `Location:` line for the path. info!(log, "Searching for installations on your machine, this is done once and will take about 10 seconds..."); - let stdout = std::process::Command::new("system_profiler") + let stdout = new_std_command("system_profiler") .args(["SPApplicationsDataType", "-detailLevel", "mini"]) .output()? .stdout; diff --git a/cli/src/self_update.rs b/cli/src/self_update.rs index bb5d2d2249b..936c6627ac0 100644 --- a/cli/src/self_update.rs +++ b/cli/src/self_update.rs @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -use std::{fs, path::Path, process::Command}; +use std::{fs, path::Path}; use tempfile::tempdir; use crate::{ @@ -11,6 +11,7 @@ use crate::{ options::Quality, update_service::{unzip_downloaded_release, Platform, Release, TargetKind, UpdateService}, util::{ + command::new_std_command, errors::{wrap, AnyError, CodeError, CorruptDownload}, http, io::{ReportCopyProgress, SilentCopyProgress}, @@ -118,7 +119,7 @@ impl<'a> SelfUpdate<'a> { } fn validate_cli_is_good(exe_path: &Path) -> Result<(), AnyError> { - let o = Command::new(exe_path) + let o = new_std_command(exe_path) .args(["--version"]) .output() .map_err(|e| CorruptDownload(format!("could not execute new binary, aborting: {}", e)))?; diff --git a/cli/src/tunnels/code_server.rs b/cli/src/tunnels/code_server.rs index 16655533754..e17e32f8840 100644 --- a/cli/src/tunnels/code_server.rs +++ b/cli/src/tunnels/code_server.rs @@ -14,7 +14,9 @@ use crate::tunnels::paths::{get_server_folder_name, SERVER_FOLDER_NAME}; use crate::update_service::{ unzip_downloaded_release, Platform, Release, TargetKind, UpdateService, }; -use crate::util::command::{capture_command, capture_command_and_check_status, kill_tree}; +use crate::util::command::{ + capture_command, capture_command_and_check_status, kill_tree, new_script_command, +}; use crate::util::errors::{wrap, AnyError, CodeError, ExtensionInstallFailed, WrappedError}; use crate::util::http::{self, BoxedHttp}; use crate::util::io::SilentCopyProgress; @@ -587,17 +589,7 @@ impl<'a> ServerBuilder<'a> { } fn get_base_command(&self) -> Command { - #[cfg(not(windows))] - let mut cmd = Command::new(&self.server_paths.executable); - #[cfg(windows)] - let mut cmd = { - let mut cmd = Command::new("cmd"); - cmd.arg("/Q"); - cmd.arg("/C"); - cmd.arg(&self.server_paths.executable); - cmd - }; - + let mut cmd = new_script_command(&self.server_paths.executable); cmd.stdin(std::process::Stdio::null()) .args(self.server_params.code_server_args.command_arguments()); cmd diff --git a/cli/src/tunnels/control_server.rs b/cli/src/tunnels/control_server.rs index a344cc5d560..df2c8b2b820 100644 --- a/cli/src/tunnels/control_server.rs +++ b/cli/src/tunnels/control_server.rs @@ -12,6 +12,7 @@ use crate::state::LauncherPaths; use crate::tunnels::protocol::{HttpRequestParams, METHOD_CHALLENGE_ISSUE}; use crate::tunnels::socket_signal::CloseReason; use crate::update_service::{Platform, Release, TargetKind, UpdateService}; +use crate::util::command::new_tokio_command; use crate::util::errors::{ wrap, AnyError, CodeError, MismatchedLaunchModeError, NoAttachedServerError, }; @@ -1019,7 +1020,7 @@ where }; } - let mut p = tokio::process::Command::new(¶ms.command); + let mut p = new_tokio_command(¶ms.command); p.args(¶ms.args); p.envs(¶ms.env); p.stdin(pipe_if!(stdin.is_some())); @@ -1060,7 +1061,7 @@ async fn handle_spawn_cli( "requested to spawn cli {} with args {:?}", params.command, params.args ); - let mut p = tokio::process::Command::new(¶ms.command); + let mut p = new_tokio_command(¶ms.command); p.args(¶ms.args); // CLI args to spawn a server; contracted with clients that they should _not_ provide these. diff --git a/cli/src/tunnels/service_windows.rs b/cli/src/tunnels/service_windows.rs index 3d2dc9f0c55..395a707f351 100644 --- a/cli/src/tunnels/service_windows.rs +++ b/cli/src/tunnels/service_windows.rs @@ -6,13 +6,11 @@ use async_trait::async_trait; use shell_escape::windows::escape as shell_escape; use std::os::windows::process::CommandExt; -use std::{ - path::PathBuf, - process::{Command, Stdio}, -}; +use std::{path::PathBuf, process::Stdio}; use winapi::um::winbase::{CREATE_NEW_PROCESS_GROUP, DETACHED_PROCESS}; use winreg::{enums::HKEY_CURRENT_USER, RegKey}; +use crate::util::command::new_std_command; use crate::{ constants::TUNNEL_ACTIVITY_NAME, log, @@ -54,7 +52,7 @@ impl CliServiceManager for WindowsService { let key = WindowsService::open_key()?; let mut reg_str = String::new(); - let mut cmd = Command::new(&exe); + let mut cmd = new_std_command(&exe); reg_str.push_str(shell_escape(exe.to_string_lossy()).as_ref()); let mut add_arg = |arg: &str| { @@ -102,7 +100,7 @@ impl CliServiceManager for WindowsService { // Start as a hidden subprocess to avoid showing cmd.exe on startup. // Fixes https://github.com/microsoft/vscode/issues/184058 // I also tried the winapi ShowWindow, but that didn't yield fruit. - Command::new(std::env::current_exe().unwrap()) + new_std_command(std::env::current_exe().unwrap()) .args(std::env::args().skip(1)) .env(DID_LAUNCH_AS_HIDDEN_PROCESS, "1") .stderr(Stdio::null()) diff --git a/cli/src/tunnels/wsl_detect.rs b/cli/src/tunnels/wsl_detect.rs index ec386feb26e..0701a89033a 100644 --- a/cli/src/tunnels/wsl_detect.rs +++ b/cli/src/tunnels/wsl_detect.rs @@ -12,7 +12,9 @@ pub fn is_wsl_installed(_log: &log::Logger) -> bool { #[cfg(windows)] pub fn is_wsl_installed(log: &log::Logger) -> bool { - use std::{path::PathBuf, process::Command}; + use std::path::PathBuf; + + use crate::util::command::new_std_command; let system32 = { let sys_root = match std::env::var("SystemRoot") { @@ -37,7 +39,7 @@ pub fn is_wsl_installed(log: &log::Logger) -> bool { // Windows builds >= 22000 let maybe_wsl = system32.join("wsl.exe"); if maybe_wsl.exists() { - if let Ok(s) = Command::new(maybe_wsl).arg("--status").output() { + if let Ok(s) = new_std_command(maybe_wsl).arg("--status").output() { if s.status.success() { trace!(log, "wsl availability detected via subprocess"); return true; diff --git a/cli/src/util/command.rs b/cli/src/util/command.rs index ad1f3a1d13e..fb9fb4f91d5 100644 --- a/cli/src/util/command.rs +++ b/cli/src/util/command.rs @@ -57,7 +57,7 @@ where I: IntoIterator, S: AsRef, { - Command::new(&command_str) + new_tokio_command(&command_str) .args(args) .stdin(Stdio::null()) .stdout(Stdio::piped()) @@ -70,15 +70,64 @@ where }) } +/// Makes a new Command, setting flags to avoid extra windows on win32 +#[cfg(windows)] +pub fn new_tokio_command(exe: impl AsRef) -> Command { + let mut p = tokio::process::Command::new(exe); + p.creation_flags(winapi::um::winbase::CREATE_NO_WINDOW); + p +} + +/// Makes a new Command, setting flags to avoid extra windows on win32 +#[cfg(not(windows))] +pub fn new_tokio_command(exe: impl AsRef) -> Command { + tokio::process::Command::new(exe) +} + +/// Makes a new command to run the target script. For windows, ensures it's run +/// in a cmd.exe context. +#[cfg(windows)] +pub fn new_script_command(script: impl AsRef) -> Command { + let mut cmd = new_tokio_command("cmd"); + cmd.arg("/Q"); + cmd.arg("/C"); + cmd.arg(script); + cmd +} + +/// Makes a new command to run the target script. For windows, ensures it's run +/// in a cmd.exe context. +#[cfg(not(windows))] +pub fn new_script_command(script: impl AsRef) -> Command { + new_tokio_command(script) // it's assumed scripts are already +x and don't need extra handling +} + +/// Makes a new Command, setting flags to avoid extra windows on win32 +#[cfg(windows)] +pub fn new_std_command(exe: impl AsRef) -> std::process::Command { + let mut p = std::process::Command::new(exe); + std::os::windows::process::CommandExt::creation_flags( + &mut p, + winapi::um::winbase::CREATE_NO_WINDOW, + ); + p +} + +/// Makes a new Command, setting flags to avoid extra windows on win32 +#[cfg(not(windows))] +pub fn new_std_command(exe: impl AsRef) -> std::process::Command { + std::process::Command::new(exe) +} + /// Kills and processes and all of its children. -#[cfg(target_os = "windows")] +#[cfg(windows)] pub async fn kill_tree(process_id: u32) -> Result<(), CodeError> { capture_command("taskkill", &["/t", "/pid", &process_id.to_string()]).await?; Ok(()) } /// Kills and processes and all of its children. -#[cfg(not(target_os = "windows"))] +#[cfg(not(windows))] pub async fn kill_tree(process_id: u32) -> Result<(), CodeError> { use futures::future::join_all; use tokio::io::{AsyncBufReadExt, BufReader};