8000 don't needlessly pass `-l <username>` or `-p <port>` to ssh by LucaFilipozzi · Pull Request #11431 · PowerShell/PowerShell · GitHub
[go: up one dir, main page]
Skip to content

don't needlessly pass -l <username> or -p <port> to ssh #11431

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
LucaFilipozzi wants to merge 7 commits into from
Closed

don't needlessly pass -l <username> or -p <port> to ssh #11431

LucaFilipozzi wants to merge 7 commits into from

Conversation

@LucaFilipozzi
Copy link
Contributor
@LucaFilipozzi LucaFilipozzi commented Dec 24, 2019 •
edited
Loading

PR Summary

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

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

PR Context

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

Similarly, StartSSHProcess passes -p <port> to ssh even when -Port <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 <username> and -p <port>, 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.

@ghost ghost assigned TravisEz13 Dec 24, 2019
@msftclas
Copy link
msftclas commented Dec 24, 2019 •
edited
Loading

CLA assistant check
All CLA requirements met.

@LucaFilipozzi LucaFilipozzi changed the title don't pass -l login_name to ssh unless -UserName is passed to cmdlets don't pass -l <username> to ssh unless -UserName <username> is passed to cmdlets Dec 24, 2019
@LucaFilipozzi LucaFilipozzi changed the title don't pass -l <username> to ssh unless -UserName <username> is passed to cmdlets don't pass -l <username> to ssh unless -UserName <username> is passed to *PSSession cmdlets Dec 24, 2019
@ghost ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Dec 25, 2019
@ghost ghost removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Dec 25, 2019
@LucaFilipozzi LucaFilipozzi changed the title don't pass -l <username> to ssh unless -UserName <username> is passed to *PSSession cmdlets don't pass -l <username> or -p <port> to ssh unless -UserName <username> or -Port <port> are passed to *PSSession cmdlets Dec 25, 2019
@LucaFilipozzi LucaFilipozzi changed the title don't pass -l <username> or -p <port> to ssh unless -UserName <username> or -Port <port> are passed to *PSSession cmdlets don't needlessly pass -l <username> or -p <port> to ssh Dec 25, 2019
@LucaFilipozzi LucaFilipozzi mentioned this pull request Dec 25, 2019
Closed
@iSazonov iSazonov added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Dec 25, 2019
@iSazonov iSazonov added this to the 7.1.0-preview.1 milestone Dec 25, 2019
LucaFilipozzi
LucaFilipozzi commented Dec 25, 2019
View reviewed changes
Copy link
Contributor Author
@LucaFilipozzi LucaFilipozzi left a comment

Choose a reason for hiding this comment

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

I have actioned the requested changes in #11432.

@LucaFilipozzi
Copy link
Contributor Author
LucaFilipozzi commented Dec 31, 2019

Let me know if you need me to make any additional changes. I believe that I've addressed the requested changes.

@TravisEz13
Copy link
Member
TravisEz13 commented Jan 7, 2020

@PaulHigin Can you review?

@ghost ghost added Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept and removed Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept labels Jan 7, 2020
PaulHigin
PaulHigin suggested changes Jan 7, 2020
View reviewed changes
@ghost ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Jan 7, 2020
@LucaFilipozzi
Merge pull request #1 from PowerShell/master
767232b 
Update local repository from origin.
@ghost ghost removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Jan 8, 2020
@LucaFilipozzi
Copy link
Contributor Author
LucaFilipozzi commented Jan 8, 2020 •
edited
Loading

Apologies for pulling in additional reviewers in my misguided attempt to bring LucaFIlipozzi/PowerShell up to date with PowerShell/PowerShell.

I'm going to close this Pull Request and create a new one.

@LucaFilipozzi LucaFilipozzi mentioned this pull request Jan 8, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SteveL-MSFT SteveL-MSFT SteveL-MSFT requested changes

@adityapatwardhan adityapatwardhan Awaiting requested review from adityapatwardhan

@anmenaga anmenaga Awaiting requested review from anmenaga

@daxian-dbw daxian-dbw Awaiting requested review from daxian-dbw daxian-dbw is a code owner

@TravisEz13 TravisEz13 Awaiting requested review from TravisEz13 TravisEz13 is a code owner

+1 more reviewer

@PaulHigin PaulHigin PaulHigin requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

@TravisEz13 TravisEz13

Labels

CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log

Projects

None yet

Milestone

7.1.0-preview.1

Development

Successfully merging this pull request may close these issues.

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

6 participants

@LucaFilipozzi @msftclas @TravisEz13 @SteveL-MSFT @PaulHigin @iSazonov
0