Skip to content

Commit

Permalink
fix: Honor disallow_shell in SSH client feature check
Browse files Browse the repository at this point in the history
When running an SSH client, the `disallow_shell` option determines
whether the client command, before arguments, is to be run directly
or if it is to be run by a shell.

(One example of when it is run directly is if it comes from the
`GIT_SSH` environment variable, while one example of when it is run
by a shell is if it comes from the `GIT_SSH_COMMAND` environment
variable.)

When invoking the client in the most central and common case of
actually attempting to connect to a remote server, `disallow_shell`
was already followed. However, in some cases we are not sure what
kind of SSH client program we have, and so to find that out (so we
know how to run it to connect to a server), we run a test command,
to see if it recognizes `-G` as OpenSSH clients do. Often we can
tell what kind of client program we have without needing to do
that. But if we do need to do it, we pre-run the client to check.
In this use, the `disallow_shell` option was not followed, and
instead the use of a shell was unconditionally treated as allowed.

This fixes that by setting `prepare.use_shell = false` on a
constructed `gix_command::Prepare` instance, which seems to be the
prevailing style for achieving this elsewhere in `gix-transport`.
  • Loading branch information
EliahKagan committed Jan 26, 2025
1 parent 38a0d9a commit a445b72
Showing 1 changed file with 8 additions and 4 deletions.
12 changes: 8 additions & 4 deletions gix-transport/src/client/blocking_io/ssh/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,8 @@ pub fn connect(
let ssh_cmd = options.ssh_command();
let mut kind = options.kind.unwrap_or_else(|| ProgramKind::from(ssh_cmd));
if options.kind.is_none() && kind == ProgramKind::Simple {
let mut cmd = std::process::Command::from(
gix_command::prepare(ssh_cmd)
let mut cmd = std::process::Command::from({
let mut prepare = gix_command::prepare(ssh_cmd)
.stderr(Stdio::null())
.stdout(Stdio::null())
.stdin(Stdio::null())
Expand All @@ -122,8 +122,12 @@ pub fn connect(
Usable(host) => host,
Dangerous(host) => Err(Error::AmbiguousHostName { host: host.into() })?,
Absent => panic!("BUG: host should always be present in SSH URLs"),
}),
);
});
if options.disallow_shell {
prepare.use_shell = false;
}
prepare
});
gix_features::trace::debug!(cmd = ?cmd, "invoking `ssh` for feature check");
kind = if cmd.status().ok().is_some_and(|status| status.success()) {
ProgramKind::Ssh
Expand Down

0 comments on commit a445b72

Please sign in to comment.