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.

The fix itself is in a445b72, but by itself this makes the connect function larger and more complicated, so 9255ccc and 9599895 refactor to address that.

I posted #1800 (comment) as a comment in a related (but not extremely tightly related) review thread, where it came up. I posted it there rather than as an issue, on the grounds that I was not done investigating it and could not, at the moment, confidently supply instructions to demonstrate the bug or a test case that would fail due to it.

After doing that, I figured that having an issue might help it advance, even if I couldn't immediately make it as complete as I would like. But then I thought, even better than an incomplete issue would be an incomplete pull request. This is that incomplete pull request. The main and possibly only way it is incomplete is that it has no tests. However, if this is judged not to require tests, then maybe this is complete after all.

While the goal of the refactoring in 9255ccc was just to avoid making the code any harder to read and understand, it might lead to making it easier to understand, since being contained in a function allows the logic to be refactored in more ways. In #1800 (comment), I had said:

[B]efore this connect function does that, it figures out how the SSH client will be invoked to do that. It checks if this is configured, and otherwise tries to ascertain what SSH implementation it is or behaves like. Under some conditions, it runs the SSH client in order to do this. I am unclear at this time on how often or exactly under what circumstances that happens.

That might become clearer with further refactoring, which should now be easier to do. However, I haven't included that in this pull request. Instead doing only as much refactoring as seemed necessary to me to avoid making the code harder to read. (Also, perhaps you just know exactly what's going on there.)
 
Please note that, in addition to being possibly incomplete, this pull request is narrowly scoped. Its goal is only to fix the place in gix-transport where the disallow_shell option is not honored. The matter of how backslashes are common and seem to get in the way of avoiding using shells is not addressed here: this does not modify gix-command.