Skip to content

Conversation

@LucaFilipozzi
Copy link
Contributor

PR Summary

Fix #11344 by not passing -l to ssh unless -UserName is passed to *PSSession cmdlets.

Fix #11432 by not passing -p to ssh unless -Port is passed to *PSSession cmdlets.

PR Context

When invoking *PSSession cmdlets, PowerShell's StartSSHProcess function passes a -l command line argument to the underlying operating system's ssh client even when -UserName is not passed to the cmdlets.

Similarly, StartSSHProcess passes -p to ssh even when -Port is not passed to the cmdlets.

Neither is necessary since the ssh client will determine the username from the environment and defaults the port to 22.

More importantly, by always passing -l and -p , StartSSHProcess overrides the User and Port directives in ssh_config should they have been provided by the system administrator.

This pull request should be non-breaking.

This pull request replaces #11431 which I messed up.

Copy link
Contributor

@PaulHigin PaulHigin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
Thanks for fixing this.

@iSazonov iSazonov added this to the 7.1.0-preview.1 milestone Jan 8, 2020
@iSazonov iSazonov added the CL-Engine Indicates that a PR should be marked as an engine change in the Change Log label Jan 8, 2020
@LucaFilipozzi
Copy link
Contributor Author

What's the next step required in merging this pull request? I see that @PaulHigin has approved (thanks!) but that doesn't qualify as an approving review?

IOW: is there anything else you need from me?

@PaulHigin
Copy link
Contributor

@PoshChan Please retry all

@PoshChan
Copy link
Collaborator

PoshChan commented Jan 9, 2020

@PaulHigin, successfully started retry of PowerShell-CI-static-analysis, PowerShell-CI-Windows, PowerShell-CI-macOS, PowerShell-CI-Linux

@PaulHigin
Copy link
Contributor

@LucaFilipozzi You don't need to do anything right now. It looks like help update tests all failed so I have restarted tests. The maintainer, @adityapatwardhan, still needs to approve and merge the changes.

@adityapatwardhan adityapatwardhan merged commit 6c792d7 into PowerShell:master Jan 29, 2020
@adityapatwardhan
Copy link
Member

@LucaFilipozzi Thank you for your contribution!

@daxian-dbw daxian-dbw modified the milestones: GA-consider, GA-approved Jan 31, 2020
@TravisEz13 TravisEz13 modified the milestones: GA-approved, 7.0.0 Feb 8, 2020
adityapatwardhan pushed a commit to adityapatwardhan/PowerShell that referenced this pull request Feb 18, 2020
@ghost
Copy link

ghost commented Feb 21, 2020

🎉v7.0.0-rc.3 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-Engine Indicates that a PR should be marked as an engine change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

don't override Port in ssh_config don't override User in ssh_config

7 participants