ssh clone does not correctly detect the location of ssh.exe

[Mr-Ao-Dragon]

Current behavior 😯

Expected behavior 🤔

No response

Git behavior

No response

Steps to reproduce 🕹

No response

[Byron]

Could you also show what Git does with git clone …? It might help to run it with GIT_TRACE=1.
Additionally, it would be good to see SSH related configuration of the output of gix config | grep -i ssh, if any is available (but probably there isn't).

Oh, and learning where git.exe and ssh.exe are located would also help.

Otherwise I could imagine that it finds ssh.exe somehow and all by itself, but in the wrong place:

https://github.com/Byron/gitoxide/blob/3e5c974dd62ac134711c6c2f5a5490187a6ea55e/gix-command/src/lib.rs#L221

The reason it has to do its own PATH lookup is to prevent Windows to launch executables that are in the current working directory, which is a blunder

My feeling is that it should be able to find ssh.exe from git --exec-path, but doesn't because it only searches PATH.

Also CC @EliahKagan

[EliahKagan]

General Windows, Rust, and gitoxide path search subtleties

The reason it has to do its own PATH lookup is to prevent Windows to launch executables that are in the current working directory, which is a blunder

It's true that the underlying CreateProcessW function will search in the current directory before searching in PATH when running a program specified with no \. Fortunately, since rust-lang/rust#87704, std::process::Command has performed its own path search to resolve full paths, so that what actually gets passed to CreateProcessW is safe.

(That fix came in with Rust 1.58.0, per the milestone linked from the PR and the tags listed as having the merge commit. The minimum supported Rust version for gitoxide is currently 1.67.0.)

For this reason, I am not sure if any of the manual PATH searching done in gitoxide is actually needed for security. However, I am reluctant to rush to change it, because:

1. The rules gix_command uses, when it does its own PATH search, differ both from the std::process::Command rules and the CreateProcessW rules. In addition to often searching in the current directory, CreateProcessW searches various other directories besides those in PATH that, unlike the current directory, are typically safe to search, though sometimes surprising. std::process::Command preserves most of that behavior. gix_command searches only the directories listed in PATH. The std::process::Command rules are probably the best way to go for Rust code where there is not a specific reason to do otherwise, but I would still want to fully understand the ramifications of changing the gix_command behavior in this area.

2. There may be other or related considerations specific to parsing #! lines in scripts that would need to be preserved across any such changes. That's not relevant to gix_transport and SSH, but it's part of what the gix_command crate has functionality for.

3. Even without letting std::process::Command take care of the lookup, gix_command still uses std::process::Command to run the command, rather than using CreateProcessW directly. Thus, I believe gix_command already benefits from other security enhancements in std::process::Command, such as the fix for CVE-2024-24576 (GHSA-q455-m56c-85mh).

Depending on what turns out to be going on in this issue, all the above might be peripheral. Either way, I should probably post something separate about this. If it's just about how things are good and don't need to be changed, I can make a discussion post; if it's about how maybe things should be changed to simplify some code in gitoxide and bring gix_command path search rules in line with std::process::Command, I can make an issue. Not having figured out which is the main reason I haven't posted anything for this yet.

We don't find ssh the same way as Git for Windows, and we probably shouldn't

My feeling is that it should be able to find ssh.exe from git --exec-path, but doesn't because it only searches PATH.

I don't think the SSH client path can be gotten directly from git --exec-path, because that only reveals the location of the git-core directory that the git executable being run uses. A full installation of Git for Windows will have various executables that can usually be found relative to this, including its own ssh executable that ships with it. This is not in the PATH used outside of the Git Bash environment, but it is what Git for Windows uses by default, even if another ssh executable is present in the PATH in the environment where git is invoked. So perhaps this is what you mean.

But I would not recommend that gitoxide default to using the ssh that ships with Git for Windows, when that is not present in PATH. gitoxide does not currently do this--though of course it will find that ssh if run in an environment where it is first in PATH, such as in Git Bash. I am somewhat inclined to say gitoxide should (continue to) give no special treatment or recognition to Git for Windows's bundled ssh at all. But I especially don't recommend changing gitoxide to prefer it over a more readily available ssh. My rationale is:

1. Although gitoxide will use Git to obtain information about configuration, when present, it does not rely on Git being present. Since gitoxide does not run Git to actually perform transport commands, it would be extremely unintuitive and strange for gitoxide to use the ssh provided as part of Git for Windows, if that is different from the ssh that is run when the user runs ssh.

2. I am not aware of any strong reasons other than backward compatibility for Git itself to prefer its own bundled ssh to another ssh in the PATH used to run it. Backward compatibility is a strong reason, for Git, and so I am unsure if Git should change this behavior. But the ssh command it bundles is far less convenient than the ssh command that is usually present in all currently supported versions of Windows. The latter integrates with a version of ssh-agent that is installed as a Windows service, can be controlled in services.msc (or with the net command), and integrates with Windows credential management features to simplify decrypting stored private keys.

3. The ssh that ships with Git for Windows seems, fairly or not, to be disfavored by users, even in the absence of an OpenSSH build that integrates with Windows. Before such a build was widely available, I believe the more common practice than using the bundled ssh, among Git for Windows users who used SSH heavily and needed an SSH agent, was to use PuTTY, with its agent, Pageant.

Of course, when the GIT_SSH environment variable is set, gitoxide should respect that, as Git does.

Troubleshooting this issue

The ssh that comes with Windows is ssh.exe in the OpenSSH subdirectory of the System32 directory. This path is most often, though not always, C:\Windows\System32\OpenSSH\ssh.exe. The error message shown in this issue seems to refer to it, but something odd is going on with the backslashes:

Error: failed to invoke program "C:\\\\Windows\\\\System32\\\\OpenSSH\\\\ssh.exe"

At most I would expect each backslash to be shown escaped by one extra backslash, such as if this is a string whose debug representation is being shown. Then each backslash in the path would be shown as \\. But this shows \\\\ for each one. So maybe that has something to do with what is going wrong.

Another curious thing is that (Get-Command ssh).Path shows:

C:\Program Files\OpenSSH\ssh.exe

That's not inherently surprising, since another version of OpenSSH may have been installed there, or perhaps the usual ssh.exe in System32\OpenSSH is absent and so this is the only installation of it. The question is why gix clone is not using this SSH.

I recommend checking the GIT_SSH environment variable, as well as the GIT_SSH_COMMAND environment variable. For example, if GIT_SSH is set to C:\Windows\System32\OpenSSH\ssh.exe, then that would explain why that ssh.exe and not the one under Program Files is being used, though I'm still not sure what the deal is with the quadruple backslashes. If GIT_SSH is set to something that actually has doubled literal backslashes, then that's the problem.

If GIT_SSH is not the problem but GIT_SSH_COMMAND is not set (and the core.sshCommand configuration variable is not set), then it may be possible to work around this problem by setting GIT_SSH, though of course there would still be an underlying gitoxide bug, or configuration problem.

[Byron]

Thanks a lot for researching this, @EliahKagan. Your help is much appreciated (as always :)).

Depending on what turns out to be going on in this issue, all the above might be peripheral. Either way, I should probably post something separate about this. If it's just about how things are good and don't need to be changed, I can make a discussion post; if it's about how maybe things should be changed to simplify some code in gitoxide and bring gix_command path search rules in line with std::process::Command, I can make an issue. Not having figured out which is the main reason I haven't posted anything for this yet.

Thanks for researching this! If memory serves, gix-command implements how Git executes hooks and sub-commands, with special handling that is implemented on Windows. Thus I think this handling should generally stay, but of course, should also be unsurprising.

This is not in the PATH used outside of the Git Bash environment, but it is what Git for Windows uses by default, even if another ssh executable is present in the PATH in the environment where git is invoked. So perhaps this is what you mean.

Yes, this is what I meant. gitoxide already uses the git installation directory to obtain additional configuration which is required for invoking credential helpers, and I think it should act like Git in other ways as well, for instance, when finding the correct ssh.exe.

[..] it would be extremely unintuitive and strange for gitoxide to use the ssh provided as part of Git for Windows, if that is different from the ssh that is run when the user runs ssh.

I agree, but also think that gitoxide has to strive for compatibility. Thus, as fallback, it should be able to find the bundled ssh command.
gix-command certainly is the right place to add such handling if it is opt-in.

Of course, when the GIT_SSH environment variable is set, gitoxide should respect that, as Git does.

And it does. You might find interesting that it reads all environment variables that map to git configuration on repository initialization, creating environment overrides in the configuration accordingly.

https://github.com/Byron/gitoxide/blob/d2ae9d5f11be9f2561f6799d88804d0d8eae33ef/gix/src/config/cache/init.rs#L593

That way environment variables that affect the runtime then become explicit:

GIT_SSH=ssh-without-shell GIT_SSH_COMMAND=ssh-override gix config
# From 'memory' (EnvOverride)
[..]
# From 'memory' (EnvOverride)
[gitoxide "ssh"]
        commandWithoutShellFallback = ssh-without-shell # from GIT_SSH

[core]
        sshCommand = ssh-override # from GIT_SSH_COMMAND

---

@Mr-Ao-Dragon Could you check if the environment variables mentioned above are set to a value and if so, post it here? Finally, could you see if you can invoke C:\\Windows\\System32\\OpenSSH\\ssh.exe and C:\Windows\System32\OpenSSH\ssh.exe respectively? Thank you.

[Mr-Ao-Dragon]

ok, i will try it

[Mr-Ao-Dragon]

I've set it up based on this blog
power session rec file

[Byron]

Apologies, but I don't think any of my questions was answered.

[Mr-Ao-Dragon]

Apologies, but I don't think any of my questions was answered.

I actually don't know what's going on, I tried the diagnostic commands you guys provided about gix but that didn't do anything, it's strange, gix does seem to detect the path but it seems to be ambiguously on the path, I found it detects more paths with more \\

[Skgland]

I've set it up based on this blog power session rec file

Attempting to playback the linked asciinema/PowerSession recording looks broken

[Byron]

I didn't try it, but maybe it only works in a powershell? Even though it aims to be compatible to Asciinema, I have a feeling that it will not play similarly in every terminal. Personally, I didn't try to play it back at all, too many hoops for me.

In a way, this issue seems stalled in terms of acquisition of information, but I hope that maybe similar issues arise with others who might chime in. In any case, I am sure eventually we will get to the bottom of this.

[Skgland]

I didn't try it, but maybe it only works in a powershell? Even though it aims to be compatible to Asciinema, I have a feeling that it will not play similarly in every terminal. Personally, I didn't try to play it back at all, too many hoops for me.

I tried both. In Bash using asciinema and in PowerShell using PowerSession. Both through Windows Terminal, though the former in WSL2. Both had identical results.

[EliahKagan]

Attempting to read the raw PowerSession log makes it look like the extra backslashes are really present in the configured value, but I am far from sure. Furthermore, repeated directory separators should not keep a path from working, so even if that is the case, it is not itself sufficient to dismiss this issue.

However, with only the information here so far, I think it is unclear that there is really a bug. Furthermore, if there is a bug, and it is later fixed, I think it would be hard even to know it was the same bug as described here. Likewise, if a bug is fixed that seems to match this, that might still be a different bug.

If this configuration is working with git, or of there is some way to provide a readable log that shows the problem and shows that it is not due to an incorrect configuration, then that would be different.

---

Instead of recording, I recommend copying the text from the terminal, and pasting it into the issue description (or into a new comment).

• If this is Windows PowerShell then the way you would copy the text depends what console application is being used. But in the old traditional console host program, copying can be achieved by opening the PowerShell window's system menu, by clicking it on the upper left corner of the window or by pressing Alt+Spacebar, and doing Edit > Select all, then again opening the system menu and doing Edit > Copy. (There are other ways to do it, but this way has the advantage that it does not produce strange or misleading results even if it is accidentally attempted in an application other than the old console host.)

• If you are using Windows PowerShell in Windows Terminal rather than the old traditional console host, then copying is easier: you can select text in the terminal, and then press Ctrl+Shift+C. That is, add a Shift to the usual key combination. (This is because Ctrl+C instead sends a signal to interrupt the program running in the terminal, so using that, without Shift, is not always effective at copying.)

• Once copied, the text can be pasted into most applications, including web browsers, in the usual way (Ctrl+V). When editing an issue description or comment, it can be presented with correct and readable formatting by putting it in a code block with the Markdown code fence feature, achieved with ``` lines at the beginning and end:

  This text is before the code block.
  
  ```
  This text is inside the code block.
  
  Text from a terminal will have its formatting preserved.
  
                  For example, this is indented because its leading spaces are rendered.
  
  And     these    words    have    big     gaps    between    them.
  ```
  
  This text is after the code block.

  That will look like:

  This text is before the code block.

  This text is inside the code block.
  
  Text from a terminal will have its formatting preserved.
  
                  For example, this is indented because its leading spaces are rendered.
  
  And     these    words    have    big     gaps    between    them.
  

  This text is after the code block.

Images, videos, and similar representations of terminal actions or contents tend to be much more difficult to use for debugging purposes than the actual text from the terminal. Copying and pasting the text is, in my opinion, by far the best way to share it, in nearly all situations.

However, if for some reason none of that is possible or you otherwise strongly prefer to present the playback of a recording, then you could upload the recording linked in #1432 (comment) (or a new one) to asciinema.org, much as you had uploaded the first one.

You may already have done this, but if so, that is not readily discoverable, because your account there currently does not have any public recordings (that is, other people need the link to see it).

[Mr-Ao-Dragon]

好的，我会尝试一下

[Byron]

I just wanted to share some insights of @EliahKagan here as they provide an avenue towards attempting a fix:

For example, when git runs a SSH client from GIT_SSH, it never uses a shell, and when it runs a SSH client command from GIT_SSH_COMMAND (or core.sshCommand), it always uses a shell. This is documented and I think the git documentation intends that users be able to rely not just on GIT_SSH_COMMAND using a shell, but also on GIT_SSH either not using a shell or behaving impeccably as though it does not. I think that gitoxide not always adhering to this may actually be the cause of #1432.

In summary, assuring that ssh is launched with or without shell depending on where the command is coming from is vital to correctness.