Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit0862dd0

Browse files
committed
fix: fix handling coder path with spaces in config-ssh
1 parenta12429e commit0862dd0

File tree

3 files changed

+73
-13
lines changed

3 files changed

+73
-13
lines changed

‎cli/configssh.go

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -112,14 +112,19 @@ func (o sshConfigOptions) equal(other sshConfigOptions) bool {
112112
}
113113

114114
func (osshConfigOptions)writeToBuffer(buf*bytes.Buffer)error {
115-
escapedCoderBinary,err:=sshConfigExecEscape(o.coderBinaryPath,o.forceUnixSeparators)
115+
escapedCoderBinaryProxy,err:=sshConfigProxyCommandEscape(o.coderBinaryPath,o.forceUnixSeparators)
116116
iferr!=nil {
117-
returnxerrors.Errorf("escape coder binary forssh failed: %w",err)
117+
returnxerrors.Errorf("escape coder binary forProxyCommand failed: %w",err)
118118
}
119119

120-
escapedGlobalConfig,err:=sshConfigExecEscape(o.globalConfigPath,o.forceUnixSeparators)
120+
escapedCoderBinaryMatchExec,err:=sshConfigMatchExecEscape(o.coderBinaryPath)
121121
iferr!=nil {
122-
returnxerrors.Errorf("escape global config for ssh failed: %w",err)
122+
returnxerrors.Errorf("escape coder binary for Match exec failed: %w",err)
123+
}
124+
125+
escapedGlobalConfig,err:=sshConfigProxyCommandEscape(o.globalConfigPath,o.forceUnixSeparators)
126+
iferr!=nil {
127+
returnxerrors.Errorf("escape global config for ProxyCommand failed: %w",err)
123128
}
124129

125130
rootFlags:=fmt.Sprintf("--global-config %s",escapedGlobalConfig)
@@ -155,7 +160,7 @@ func (o sshConfigOptions) writeToBuffer(buf *bytes.Buffer) error {
155160
_,_=buf.WriteString("\t")
156161
_,_=fmt.Fprintf(buf,
157162
"ProxyCommand %s %s ssh --stdio%s --ssh-host-prefix %s %%h",
158-
escapedCoderBinary,rootFlags,flags,o.userHostPrefix,
163+
escapedCoderBinaryProxy,rootFlags,flags,o.userHostPrefix,
159164
)
160165
_,_=buf.WriteString("\n")
161166
}
@@ -174,11 +179,11 @@ func (o sshConfigOptions) writeToBuffer(buf *bytes.Buffer) error {
174179
// the ^^ options should always apply, but we only want to use the proxy command if Coder Connect is not running.
175180
if!o.skipProxyCommand {
176181
_,_=fmt.Fprintf(buf,"\nMatch host *.%s !exec\"%s connect exists %%h\"\n",
177-
o.hostnameSuffix,escapedCoderBinary)
182+
o.hostnameSuffix,escapedCoderBinaryMatchExec)
178183
_,_=buf.WriteString("\t")
179184
_,_=fmt.Fprintf(buf,
180185
"ProxyCommand %s %s ssh --stdio%s --hostname-suffix %s %%h",
181-
escapedCoderBinary,rootFlags,flags,o.hostnameSuffix,
186+
escapedCoderBinaryProxy,rootFlags,flags,o.hostnameSuffix,
182187
)
183188
_,_=buf.WriteString("\n")
184189
}
@@ -759,7 +764,8 @@ func sshConfigSplitOnCoderSection(data []byte) (before, section []byte, after []
759764
returndata,nil,nil,nil
760765
}
761766

762-
// sshConfigExecEscape quotes the string if it contains spaces, as per
767+
// sshConfigProxyCommandEscape prepares the path for use in ProxyCommand.
768+
// It quotes the string if it contains spaces, as per
763769
// `man 5 ssh_config`. However, OpenSSH uses exec in the users shell to
764770
// run the command, and as such the formatting/escape requirements
765771
// cannot simply be covered by `fmt.Sprintf("%q", path)`.
@@ -804,7 +810,7 @@ func sshConfigSplitOnCoderSection(data []byte) (before, section []byte, after []
804810
// This is a control flag, and that is ok. It is a control flag
805811
// based on the OS of the user. Making this a different file is excessive.
806812
// nolint:revive
807-
funcsshConfigExecEscape(pathstring,forceUnixPathbool) (string,error) {
813+
funcsshConfigProxyCommandEscape(pathstring,forceUnixPathbool) (string,error) {
808814
ifforceUnixPath {
809815
// This is a workaround for #7639, where the filepath separator is
810816
// incorrectly the Windows separator (\) instead of the unix separator (/).
@@ -814,9 +820,9 @@ func sshConfigExecEscape(path string, forceUnixPath bool) (string, error) {
814820
// This is unlikely to ever happen, but newlines are allowed on
815821
// certain filesystems, but cannot be used inside ssh config.
816822
ifstrings.ContainsAny(path,"\n") {
817-
return"",xerrors.Errorf("invalid path: %s",path)
823+
return"",xerrors.Errorf("invalid path: %q",path)
818824
}
819-
// In the unlikelyeven that a path contains quotes, they must be
825+
// In the unlikelyevent that a path contains quotes, they must be
820826
// escaped so that they are not interpreted as shell quotes.
821827
ifstrings.Contains(path,"\"") {
822828
path=strings.ReplaceAll(path,"\"","\\\"")

‎cli/configssh_internal_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ func Test_sshConfigExecEscape(t *testing.T) {
171171
err=os.WriteFile(bin,contents,0o755)//nolint:gosec
172172
require.NoError(t,err)
173173

174-
escaped,err:=sshConfigExecEscape(bin,false)
174+
escaped,err:=sshConfigProxyCommandEscape(bin,false)
175175
iftt.wantErr {
176176
require.Error(t,err)
177177
return
@@ -236,7 +236,7 @@ func Test_sshConfigExecEscapeSeparatorForce(t *testing.T) {
236236
tt:=tt
237237
t.Run(tt.name,func(t*testing.T) {
238238
t.Parallel()
239-
found,err:=sshConfigExecEscape(tt.path,tt.forceUnix)
239+
found,err:=sshConfigProxyCommandEscape(tt.path,tt.forceUnix)
240240
iftt.wantErr {
241241
require.Error(t,err)
242242
return

‎cli/configssh_windows.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,59 @@
22

33
package cli
44

5+
import (
6+
"fmt"
7+
"strings"
8+
9+
"golang.org/x/xerrors"
10+
)
11+
512
// Must be a var for unit tests to conform behavior
613
varhideForceUnixSlashes=false
14+
15+
// sshConfigMatchExecEscape prepares the path for use in `Match exec` statement.
16+
//
17+
// OpenSSH parses the Match line with a very simple tokenizer that accepts "-enclosed strings for the exec command, and
18+
// has no supported escape sequences for ". This means we cannot include " within the command to execute.
19+
//
20+
// To make matters worse, on Windows, OpenSSH passes the string directly to cmd.exe for execution, and as far as I can
21+
// tell, the only supported way to call a path that has spaces in it is to surround it with ".
22+
//
23+
// So, we can't actually include " directly, but here is a horrible workaround:
24+
//
25+
// "for /f %%a in ('powershell.exe -Command [char]34') do @cmd.exe /c %%aC:\Program Files\Coder\bin\coder.exe%%a connect exists %h"
26+
//
27+
// The key insight here is to store the character " in a variable (%a in this case, but the % itself needs to be
28+
// escaped, so it becomes %%a), and then use that variable to construct the double-quoted path:
29+
//
30+
// %%aC:\Program Files\Coder\bin\coder.exe%%a.
31+
//
32+
// How do we generate a single " character without actually using that character? I couldn't find any command in cmd.exe
33+
// to do it, but powershell.exe can convert ASCII to characters like this: `[char]34` (where 34 is the code point for ").
34+
//
35+
// Other notes:
36+
// - @ in `@cmd.exe` suppresses echoing it, so you don't get this command printed
37+
// - without another invocation of cmd.exe (e.g. `do @cmd.exe /c %%aC:\Program Files\Coder\bin\coder.exe%%a`) then
38+
// the double-quote gets interpreted as part of the path and you get: '"C:\Program' is not recognized. Constructing
39+
// the string and then passing it to another instance of cmd.exe does this trick here.
40+
// - OpenSSH passes the `Match exec` command to cmd.exe regardless of whether the user has a unix-like shell like
41+
// git bash, so we don't have a `forceUnixPath` option like for the ProxyCommand which does respect the user's
42+
// configured shell.
43+
funcsshConfigMatchExecEscape(pathstring) (string,error) {
44+
// This is unlikely to ever happen, but newlines are allowed on
45+
// certain filesystems, but cannot be used inside ssh config.
46+
ifstrings.ContainsAny(path,"\n") {
47+
return"",xerrors.Errorf("invalid path: %s",path)
48+
}
49+
// Windows does not allow double-quotes in paths. If we get one it is an error.
50+
ifstrings.Contains(path,`"`) {
51+
return"",xerrors.Errorf("path must not contain quotes: %q",path)
52+
}
53+
// A space or a tab requires quoting, but tabs must not be escaped
54+
// (\t) since OpenSSH interprets it as a literal \t, not a tab.
55+
ifstrings.ContainsAny(path,"\t") {
56+
// c.f. function comment for how this works.
57+
path=fmt.Sprintf("for /f %%%%a in ('powershell.exe -Command [char]34') do @cmd.exe /c %%%%a%s%%%%a",path)//nolint:gocritic // We don't want %q here.
58+
}
59+
returnpath,nil
60+
}

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp