Dynamically generate profiles from hosts in OpenSSH config files

[jonthysell]

trafficstars

Summary of the Pull Request

This PR adds a new dynamic profile generator which creates profiles to quickly connect to detected SSH hosts.

References

Closes #9031

PR Checklist

• [x] Closes #9031
• [x] CLA signed. If not, go over here and sign the CLA
• [x] Tests added/passed
• [ ] Documentation updated. If checked, please file a pull request on our docs repo and link it here: #xxx
• [ ] Schema updated.
• [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

Detailed Description of the Pull Request / Additional comments

This PR adds a new SshHostGenerator inbox dynamic profile generator. When run, it looks for an install of our Win32-OpenSSH client app ssh.exe in all of the (official) places it gets installed. If the exe is found, the generator then looks for and parses both the user and system OpenSSH config files for valid SSH hosts. Each host is then converted into a profiles to call ssh.exe and connect to those hosts.

Validation Steps Performed

Installed OpenSSH, configured host for alt.org NetHack server, connected and played some NetHack from the created profile.

• [x] When OpenSSH is not installed, don't add profiles
• [x] Detected when installed via Optional Features (installs in System32\OpenSSH, added to PATH)
• [x] Detected when installed via 32-Bit MSI from GitHub (installs in Program Files (x86)\OpenSSH, not added to PATH)
• [x] Detected when installed via 64-Bit MSI from GitHub (installs in Program Files\OpenSSH, not added to PATH)
• [x] Detected when installed via winget install Microsoft.OpenSSH.Beta (uses MSI from GitHub)
• [ ] ~Detected when installed via choco install openssh --pre (out-of-date, but installs to Program Files\OpenSSH-Win64 or Program Files (x86)\OpenSSH-Win32, added to PATH)~
• [x] With "disabledProfileSources": ["Windows.Terminal.SSH"] the profiles are not generated

[jonthysell]

[jonthysell]

Tests pass, but I don't see an obvious place for adding new tests (there are no existing tests for dynamic profiles).

I'm not sure what Schema needs to be updated.

Working on a paired docs PR.

[lhecker]

We've discussed this PR among the team today. We've had a history of generating too many profiles during startup which leads to usability problems when this list for instance gets to too long. For instance my list looks like that by default already when I first launch Windows Terminal:

Long term we'd like to improve this by introducing a nested new tab menu like this:

(See also #13763.)

...as well as adding a revamped settings UI which nests or groups profiles, removing them from the navigation view on the left. Finally, we've been thinking about not adding any generated profiles by default anymore and instead improve the settings UI in such a way that creating/cloning profiles will be much easier. So instead of adding all your SSH hosts all at once you can choose which ones are supposed to show up in your UI first. While the first one should be landing fairly soon as an initial implementation, we have no design or plans ready for the latter two just yet. But we believe that this PR and the unbounded list of profiles it can generate, requires these improvements to exist first, or otherwise it would swamp users with too many profiles making UI navigation too "annoying".

As such we'd like to hide it behind a feature flag for now similar to Feature_VtPassthroughModeSettingInUI which disables it by default until we can sort out our UI/UX issues. 😕

Would you agree with our reasoning? If not, please let us know! 🙂

[ffes]

Although I would LOVE to have this feature, as I asked in https://github.com/microsoft/terminal/issues/9031#issuecomment-1268946721, I am pleased with this decision. 👍🏻

With over 20 profiles in my .ssh/config I really don't want them all just added to my list of profiles. There are already 7 profiles there, and I have disabled a couple of the default ones. Having a UI to manage that would be great and having the submenus would make it perfect.

[ghost]

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment.

[vbrozik]

Although the idea of this feature is great it would be unusable for serious users of hosts inside OpenSSH config:

1. As mentioned above: It is unusable for tens of hosts - it would generate too long menu if submenus are not implemented.
2. It does not scan Include files. Includes are really useful for managing large OpenSSH configuration split for multiple subjects.

[jonthysell]

Would you agree with our reasoning? If not, please let us know! 🙂

I absolutely agree that the UI could do a lot better than just dump every possible detected profile (across all the dynamic profiles) in to the list automatically. Even the existing dynamic profiles take a little too much control out of my hands as a user. I'd much prefer being able to explicitly choose which of any detected profiles I want, and would love the option of organizing them into sub-menus.

This PR implements the most "straight-forward" path of using the existing dynamic profile infrastructure to add support for SSH profiles with as little code as possible. Obviously if you want to enhance the dynamic profile infra first, so the profiles don't spam the user, then by all means do so.

[jonthysell]

Although the idea of this feature is great it would be unusable for serious users of hosts inside OpenSSH config:

1. As mentioned above: It is unusable for tens of hosts - it would generate too long menu if submenus are not implemented.
2. It does not scan Include files. Includes are really useful for managing large OpenSSH configuration split for multiple subjects.

I was not aware of the use of Include files. Maybe worthy of a future enhancement to this code? As it stands it already looks like Terminal needs to address the potential menu spamming issue, as well as the dynamic profile bug I found before expanding Terminal with more dynamically generated profiles.

[carlos-zamora]

Alright, we chatted about this in sync today. Outside of that issue for profile management we've been chatting about above, this PR is pretty much good to go. So we're going to review/merge it as-is, but hide generating the profiles behind a feature flag.

I'll handle adding the feature flag code, so don't worry about that. :) It'll require me to push to this branch/PR though FYI,

Excited to see this land!

[vbrozik]

I was not aware of the use of Include files. Maybe worthy of a future enhancement to this code?

In my opinion support for Include files is necessary. Without that the functionality is just partial. Also when hierarchy of profiles is implemented I think every include file should have its directory in the hierarchy by default.

I think that the use of the Include directive is pretty common among serious users of ~/.ssh/config. I personally have no Host entries in the main configuration file. All of them are in the included files. Just few examples from quick googling:

• https://lcguida.com/2020/05/29/organizing-ssh_config-files/
• https://www.jamesridgway.co.uk/simplify-ssh-with-ssh-config-files/
• https://joshtronic.com/2018/09/28/include-files-in-your-ssh-config/
• https://io.adafruit.com/blog/notebook/2016/09/27/ssh-config-includes/

[zadjii-msft]

In my opinion support for Include files is necessary. Without that the functionality is just partial

FWIW, I'm not gonna hold this PR up over that. We should file a follow up, to track it, but let's not let perfect be the enemy of the good.

[zadjii-msft]

@DHowett I'm gonna remove your block tomorrow if you don't, cause we're merging the nested menu & filtering, AND cause carlos added the velocity flag.

[ghost]

Hello @DHowett!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

[DHowett]

Merge and iterate!

[DHowett]

@jonthysell thanks so much for doing this, and putting up with our quite long review cycle!

[DHowett]

@zadjii-msft @carlos-zamora somebody may need to merge main! I am not able to do so at the moment

[github-actions[bot]]

@check-spelling-bot Report

:red_circle: Please review

See the :open_file_folder: files view or the :scroll:action log for details.

Unrecognized words (3)

SYSTEMCONFIG Thysell USERCONFIG

Previously acknowledged words that are now absent

Hirots :arrow_right:

To accept :heavy_check_mark: these unrecognized words as correct and remove the previously acknowledged and now absent words, run the following commands

... in a clone of the [email protected]:jonthysell/terminal.git repository on the dynamicssh branch (:information_source: how do I use this?):

curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/v0.0.21/apply.pl' |
perl - 'https://github.com/microsoft/terminal/actions/runs/3648259624/attempts/1'

:pencil2: Contributor please read this

By default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.

:warning: The command is written for posix shells. If it doesn't work for you, you can manually add (one word per line) / remove items to expect.txt and the excludes.txt files.

If the listed items are:

• ... misspelled, then please correct them instead of using the command.
• ... names, please add them to .github/actions/spelling/allow/names.txt.
• ... APIs, you can add them to a file in .github/actions/spelling/allow/.
• ... just things you're using, please add them to an appropriate file in .github/actions/spelling/expect/.
• ... tokens you only need in one place and shouldn't generally be used, you can add an item in an appropriate file in .github/actions/spelling/patterns/.

See the README.md in each directory for more information.

:microscope: You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. :wink:

If the flagged items are :exploding_head: false positives

If items relate to a ...

• binary file (or some other file you wouldn't want to check at all).

  Please add a file path to the excludes.txt file matching the containing file.

  File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

  ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

• well-formed pattern.

  If you can write a pattern that would match it, try adding it to the patterns.txt file.

  Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

  Note that patterns can't match multiline strings.

[phil-blain]

Hi everyone, very glad to have found this PR, thanks for working on this. I've been doing this with a Python script up to now:

https://gist.github.com/phil-blain/2e5a294b79ec26a1729aedab010a9369. This uses the paramiko Python library to parse the SSH config file. A few things I do in my script that this PR does not do (at least from my reading of the changes):

• It works for both Host and Match host clauses in the SSH config file (with a patch to paramiko)
• It filters out hosts that contain a wildcard character * (since these are not real host names, just "generic" names used in the config file to group settings related to similarly named hosts together)
• It uses powershell -NoExit -Command \"ssh {host}\" as the command line, to avoid the tab being unusable if the connection closes (we are dropped back in Powershell and so can SSH back in manually)
• It uses the OpenSSH logo as icon :D (not sure if that's what "ms-appx:///ProfileIcons/{550ce7b8-d500-50ad-8a1a-c400c3262db3}.png"; is )

Just a few suggestions :)

EDIT a question I just had: Why not also simply look for ssh.exe in $PATH ? maybe someone download the zip from Win32-OpenSSH and put it in their PATH ?