Skip to content

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 9, 2024

Conversation

mafredri
Copy link
Member

@mafredri mafredri commented Jul 8, 2024

This PR changes the config-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

@mafredri mafredri self-assigned this Jul 8, 2024
@mafredri mafredri requested review from Emyrk and mtojek July 8, 2024 16:01
@mafredri mafredri force-pushed the mafredri/fix-configssh-bad-duplicate-removal branch from 65d83f2 to 8e131eb Compare July 8, 2024 16:12
@Emyrk
Copy link
Member

Emyrk commented Jul 8, 2024

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

TIL keys can be duplicated.

@mafredri
Copy link
Member Author

mafredri commented Jul 8, 2024

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

TIL keys can be duplicated.

Yeah, they're additive in the case of RemoteForward. 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).

Copy link
Member

@mtojek mtojek left a comment

Choose a reason for hiding this comment

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

👍

@mafredri mafredri merged commit 978364b into main Jul 9, 2024
28 checks passed
@mafredri mafredri deleted the mafredri/fix-configssh-bad-duplicate-removal branch July 9, 2024 06:44
@github-actions github-actions bot locked and limited conversation to collaborators Jul 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CoderCLI incorrectly removes duplicate RemoteForward entries in ssh-config
4 participants