NotificationsYou must be signed in to change notification settings
Fork928
Star10.1k

fix(cli): do not overwrite repeated SSH options in config-ssh#13819

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.

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

Already on GitHub?Sign in to your account

Jump to bottom

Merged

mafredri merged 3 commits intomainfrommafredri/fix-configssh-bad-duplicate-removal

Jul 9, 2024

Merged

fix(cli): do not overwrite repeated SSH options in config-ssh#13819

mafredri merged 3 commits intomainfrommafredri/fix-configssh-bad-duplicate-removal

Jul 9, 2024

Conversation

Copy link

Member

mafredri commentedJul 8, 2024

This PR changes theconfig-ssh such that it relies on SSH config option prioritization (first come, first served) to retain the behavior when options supporting multiple entries are defined. We also retain the ability to remove options by setting them to an empty value. Removal is perhaps the strangest part of this, and ultimately we may be better served by a different method of removing an option, but I think this is fine for now.

Fixes#11593

fix(cli): do not overwrite repeated SSH options in config-ssh

62ada21

Fixes#11593

mafredri self-assigned this

Jul 8, 2024

mafredri requested review fromEmyrk andmtojek

July 8, 2024 16:01

fix test and case insensitivity

8e131eb

mafredri force-pushed themafredri/fix-configssh-bad-duplicate-removal branch from65d83f2 to8e131ebCompare

July 8, 2024 16:12

improve comment

bec1f01

Copy link

Member

Emyrk commentedJul 8, 2024

Just to make sure I understand, this is because you can have multipleRemoteForward keys in an ssh config?

TIL keys can be duplicated.

Copy link

MemberAuthor

mafredri commentedJul 8, 2024

Just to make sure I understand, this is because you can have multipleRemoteForward keys in an ssh config?
TIL keys can be duplicated.

Yeah, they're additive in the case ofRemoteForward. Applies to a few other keys as well. And since OpenSSH handles duplications natively, I decided it's best to rely on that property (order matters for non-duplicatable keys).