Skip to content

feat: validate ssh properties before launching workspaces #96

New issue

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

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

Already on GitHub? # to your account

Merged
merged 1 commit into from
May 4, 2023

Conversation

kylecarbs
Copy link
Member

This was reported by a member in Discord, and displayed a poor error by VS Code when it failed before.

Users cannot override our config options otherwise connections may fail or have unexpected behavior.

@kylecarbs kylecarbs requested a review from mafredri May 4, 2023 17:17
@kylecarbs kylecarbs self-assigned this May 4, 2023
Copy link
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

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

Looks good, we should probably handle the = case for config options, but LGTM otherwise! 👍🏻

src/remote.ts Outdated
{
useCustom: true,
modal: true,
detail: `Your SSH config is overriding the "${key}=${computedProperties[key]}" option for the "${hostName}" host. Please remove it and try again!`,
Copy link
Member

Choose a reason for hiding this comment

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

We could add "overriding the key=correct with key=wrong ..." to help trace the issue?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point!

if (line === "") {
return
}
const [key, ...valueParts] = line.split(/\s+/)
Copy link
Member

Choose a reason for hiding this comment

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

Separator could be = or space.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ahh, great point! I'll add a test case.

@kylecarbs
Copy link
Member Author

@mafredri
image

This was reported by a member in Discord, and displayed a poor error by VS Code
when it failed before.

Users cannot override our config options otherwise connections
may fail or have unexpected behavior.
@kylecarbs kylecarbs merged commit 4c37680 into main May 4, 2023
@kylecarbs kylecarbs deleted the sshoption branch May 4, 2023 17:41
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants