remote: Fix incorrect arguments passed to SCP & SFTP (#56080)
All arguments passed to the initial remote connection command were initially blindly passed to `scp` or `sftp` command line. This is an issue with some arguments like "-X" or "-Y" that either does not exist, or have a different semantic. This PR introduces a denylist of arguments before crafting the command to execute. Reproduce steps: 1. Create a new remote connection, e.g `ssh -X user@192.168.1.1` 2. Expect the upload of the remote server to fail with a message saying that '-X' is not a valid argument for `ssh` / `sftp` depending on your setup. Self-Review Checklist: - [X] I've reviewed my own diff for quality, security, and reliability - [X] Unsafe blocks (if any) have justifying comments - [X] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [ ] Tests cover the new/changed behavior - [X] Performance impact has been considered and is acceptable Release Notes: - Fixed wrong arguments that may be passed to scp or sftp command when pushing remote files. --------- Co-authored-by: Marshall Bowers <git@maxdeviant.com> Co-authored-by: Lukas Wirth <lukas@zed.dev>
This commit is contained in:
1 file changed
+34
-18
@@ -1169,17 +1169,22 @@ impl SshRemoteConnection {
|
||||
&self,
|
||||
src_path: &Path,
|
||||
dest_path_str: &str,
|
||||
args: Option<&[&str]>,
|
||||
additional_args: Option<&[&str]>,
|
||||
) -> util::command::Command {
|
||||
/// These arguments exist for `ssh` but don't exist / don't have the same semantic for `scp`.
|
||||
const SSH_DENY_ARGS_FOR_SCP: &[&str] = &["-X", "-Y"];
|
||||
|
||||
let mut command = util::command::new_command("scp");
|
||||
self.socket.ssh_options(&mut command, false).args(
|
||||
self.socket
|
||||
.connection_options
|
||||
.port
|
||||
.map(|port| vec!["-P".to_string(), port.to_string()])
|
||||
.unwrap_or_default(),
|
||||
);
|
||||
if let Some(args) = args {
|
||||
self.socket
|
||||
.ssh_options(&mut command, false, Some(SSH_DENY_ARGS_FOR_SCP))
|
||||
.args(
|
||||
self.socket
|
||||
.connection_options
|
||||
.port
|
||||
.map(|port| vec!["-P".to_string(), port.to_string()])
|
||||
.unwrap_or_default(),
|
||||
);
|
||||
if let Some(args) = additional_args {
|
||||
command.args(args);
|
||||
}
|
||||
command.arg(src_path).arg(format!(
|
||||
@@ -1191,14 +1196,19 @@ impl SshRemoteConnection {
|
||||
}
|
||||
|
||||
fn build_sftp_command(&self) -> util::command::Command {
|
||||
// these arguments exist for "ssh" but don't exist / don't have the same semantic for "sftp"
|
||||
const SSH_DENY_ARGS_FOR_SFTP: &[&str] = &["-X", "-Y"];
|
||||
|
||||
let mut command = util::command::new_command("sftp");
|
||||
self.socket.ssh_options(&mut command, false).args(
|
||||
self.socket
|
||||
.connection_options
|
||||
.port
|
||||
.map(|port| vec!["-P".to_string(), port.to_string()])
|
||||
.unwrap_or_default(),
|
||||
);
|
||||
self.socket
|
||||
.ssh_options(&mut command, false, Some(SSH_DENY_ARGS_FOR_SFTP))
|
||||
.args(
|
||||
self.socket
|
||||
.connection_options
|
||||
.port
|
||||
.map(|port| vec!["-P".to_string(), port.to_string()])
|
||||
.unwrap_or_default(),
|
||||
);
|
||||
command.arg("-b").arg("-");
|
||||
command.arg(self.socket.connection_options.scp_destination());
|
||||
command.stdin(Stdio::piped());
|
||||
@@ -1337,7 +1347,7 @@ impl SshSocket {
|
||||
let separator = shell_kind.sequential_commands_separator();
|
||||
format!("cd{separator} {to_run}")
|
||||
};
|
||||
self.ssh_options(&mut command, true)
|
||||
self.ssh_options(&mut command, true, None)
|
||||
.arg(self.connection_options.ssh_destination());
|
||||
if !allow_pseudo_tty {
|
||||
command.arg("-T");
|
||||
@@ -1369,13 +1379,19 @@ impl SshSocket {
|
||||
&self,
|
||||
command: &'a mut util::command::Command,
|
||||
include_port_forwards: bool,
|
||||
deny_args: Option<&[&str]>,
|
||||
) -> &'a mut util::command::Command {
|
||||
let args = if include_port_forwards {
|
||||
let mut args = if include_port_forwards {
|
||||
self.connection_options.additional_args()
|
||||
} else {
|
||||
self.connection_options.additional_args_for_scp()
|
||||
};
|
||||
|
||||
// draining all arguments that are explicitly denied
|
||||
if let Some(deny_args) = deny_args {
|
||||
args.retain(|x| !deny_args.contains(&x.as_str()));
|
||||
}
|
||||
|
||||
let cmd = command
|
||||
.stdin(Stdio::piped())
|
||||
.stdout(Stdio::piped())
|
||||
|
||||
Reference in new issue
Block a user