Skip to content

Coder CLI on Windows: SSH ProxyCommand does not include quotes #7639

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
bpmct opened this issue May 22, 2023 · 5 comments · Fixed by #8164
Closed

Coder CLI on Windows: SSH ProxyCommand does not include quotes #7639

bpmct opened this issue May 22, 2023 · 5 comments · Fixed by #8164
Assignees
@Emyrk
Labels
s1 Bugs that break core workflows. Only humans may set this.
Milestone
🦛 Sprint 1

Comments

@bpmct
Copy link
Member

bpmct commented May 22, 2023

It seems like when I manually download coder.exe and add to PATH, the ProxyCommand does not include quotes which results in failed SSH connections in many cases, including when using git bash.

From manual .exe download, added to path, and coder config-ssh in git bash:

Host coder.dev
	HostName coder.dev
	ConnectTimeout=0
	StrictHostKeyChecking=no
	UserKnownHostsFile=/dev/null
	LogLevel ERROR
	ProxyCommand C:\Users\benpotter\Desktop\bin\coder.exe --global-config C:\Users\benpotter\AppData\Roaming\coderv2 ssh --stdio dev

Result when I try to SSH:

$ ssh coder.dev
/usr/bin/bash: line 1: exec: C:UsersbenpotterDesktopbincoder.exe: not found
kex_exchange_identification: Connection closed by remote host

From msi installer (this works):

Host coder.dev
	HostName coder.dev
	ConnectTimeout=0
	StrictHostKeyChecking=no
	UserKnownHostsFile=/dev/null
	LogLevel ERROR
	ProxyCommand "C:\Program Files\Coder\bin\coder.exe" --global-config C:\Users\benpotter\AppData\Roaming\coderv2 ssh --stdio dev
@kylecarbs
Copy link
Member

@bpmct if you manually toss quotes around the exe path does it work? I'll add an exception for this right away.

@bpmct
Copy link
Member Author

bpmct commented May 22, 2023

yeah, it works if i manually add the quotes.

@bpmct bpmct added bug s1 Bugs that break core workflows. Only humans may set this. labels May 25, 2023
@sreya sreya added this to the ❓Sprint 1 milestone Jun 12, 2023
@Emyrk
Copy link
Member

Emyrk commented Jun 22, 2023

There has been a deep dive here already:

coder/cli/configssh.go

Lines 703 to 729 in eee4f83

// sshConfigExecEscape quotes the string if it contains spaces, as per
// `man 5 ssh_config`. However, OpenSSH uses exec in the users shell to
// run the command, and as such the formatting/escape requirements
// cannot simply be covered by `fmt.Sprintf("%q", path)`.
//
// Always escaping the path with `fmt.Sprintf("%q", path)` usually works
// on most platforms, but double quotes sometimes break on Windows 10
// (see #2853). This function takes a best-effort approach to improving
// compatibility and covering edge cases.
//
// Given the following ProxyCommand:
//
// ProxyCommand "/path/with space/coder" ssh --stdio work
//
// This is ~what OpenSSH would execute:
//
// /bin/bash -c '"/path/with space/to/coder" ssh --stdio workspace'
//
// However, since it's actually an arg in C, the contents inside the
// single quotes are interpreted as is, e.g. if there was a '\t', it
// would be the literal string '\t', not a tab.
//
// See:
// - https://github.com/coder/coder/issues/2853
// - https://github.com/openssh/openssh-portable/blob/V_9_0_P1/sshconnect.c#L158-L167
// - https://github.com/PowerShell/openssh-portable/blob/v8.1.0.0/sshconnect.c#L231-L293
// - https://github.com/PowerShell/openssh-portable/blob/v8.1.0.0/contrib/win32/win32compat/w32fd.c#L1075-L1100

So simply adding quotes might not be a complete solution.

@Emyrk
Copy link
Member

Emyrk commented Jun 22, 2023 •
edited
Loading

Talked with @mafredri about this. Simply quoting the strings is not a complete solution.

The issue is that the git bash is being "unix compatabile", but path/filepath package is using windows path separators (being the backslash).

So I am going to add a flag + env var that allows forcing to use the forward slash. So people in this situation can run something like coder config-ssh --force-unix-filepaths or use an env var to keep it persistent.

This is an unfortunate fix. But any solution aimed at fixing this could break the windows Command prompt. So this is the stop gap.

@Emyrk Emyrk mentioned this issue Jun 22, 2023
Merged
@Emyrk
Copy link
Member

Emyrk commented Jun 22, 2023

Not great, but this can now be done on windows:

coder --config-ssh --force-unix-filepaths

Setting an env var in your profile would make this permanent

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

@Emyrk Emyrk

Labels
s1 Bugs that break core workflows. Only humans may set this.
Projects
None yet
Milestone
🦛 Sprint 1
Development

Successfully merging a pull request may close this issue.

feat: add flag for Windows to create unix compatible filepaths coder/coder
5 participants
@sreya @Emyrk @kylecarbs @matifali @bpmct