Skip to content

Commit 797e91d

Browse files
EmyrkEmyrk
authored
feat: add flag for Windows to create unix compatible filepaths (#8164)
* feat: add flag for Windows to create unix compatible filepaths
1 parent 5d45218 commit 797e91d

5 files changed

+142
-16
lines changed

‎cli/configssh.go

+46-8
Original file line numberDiff line numberDiff line change
@@ -192,11 +192,12 @@ func sshPrepareWorkspaceConfigs(ctx context.Context, client *codersdk.Client) (r
192192
//nolint:gocyclo
193193
func (r *RootCmd) configSSH() *clibase.Cmd {
194194
var (
195-
sshConfigFile string
196-
sshConfigOpts sshConfigOptions
197-
usePreviousOpts bool
198-
dryRun bool
199-
skipProxyCommand bool
195+
sshConfigFile string
196+
sshConfigOpts sshConfigOptions
197+
usePreviousOpts bool
198+
dryRun bool
199+
skipProxyCommand bool
200+
forceUnixSeparators bool
200201
)
201202
client := new(codersdk.Client)
202203
cmd := &clibase.Cmd{
@@ -236,13 +237,13 @@ func (r *RootCmd) configSSH() *clibase.Cmd {
236237
if err != nil {
237238
return err
238239
}
239-
escapedCoderBinary, err := sshConfigExecEscape(coderBinary)
240+
escapedCoderBinary, err := sshConfigExecEscape(coderBinary, forceUnixSeparators)
240241
if err != nil {
241242
return xerrors.Errorf("escape coder binary for ssh failed: %w", err)
242243
}
243244

244245
root := r.createConfig()
245-
escapedGlobalConfig, err := sshConfigExecEscape(string(root))
246+
escapedGlobalConfig, err := sshConfigExecEscape(string(root), forceUnixSeparators)
246247
if err != nil {
247248
return xerrors.Errorf("escape global config for ssh failed: %w", err)
248249
}
@@ -540,6 +541,19 @@ func (r *RootCmd) configSSH() *clibase.Cmd {
540541
Default: "auto",
541542
Value: clibase.EnumOf(&sshConfigOpts.waitEnum, "yes", "no", "auto"),
542543
},
544+
{
545+
Flag: "force-unix-filepaths",
546+
Env: "CODER_CONFIGSSH_UNIX_FILEPATHS",
547+
Description: "By default, 'config-ssh' uses the os path separator when writing the ssh config. " +
548+
"This might be an issue in Windows machine that use a unix-like shell. " +
549+
"This flag forces the use of unix file paths (the forward slash '/').",
550+
Value: clibase.BoolOf(&forceUnixSeparators),
551+
// On non-windows showing this command is useless because it is a noop.
552+
// Hide vs disable it though so if a command is copied from a Windows
553+
// machine to a unix machine it will still work and not throw an
554+
// "unknown flag" error.
555+
Hidden: hideForceUnixSlashes,
556+
},
543557
cliui.SkipPromptOption(),
544558
}
545559

@@ -727,7 +741,31 @@ func writeWithTempFileAndMove(path string, r io.Reader) (err error) {
727741
// - https://github.com/openssh/openssh-portable/blob/V_9_0_P1/sshconnect.c#L158-L167
728742
// - https://github.com/PowerShell/openssh-portable/blob/v8.1.0.0/sshconnect.c#L231-L293
729743
// - https://github.com/PowerShell/openssh-portable/blob/v8.1.0.0/contrib/win32/win32compat/w32fd.c#L1075-L1100
730-
func sshConfigExecEscape(path string) (string, error) {
744+
//
745+
// Additional Windows-specific notes:
746+
//
747+
// In some situations a Windows user could be using a unix-like shell such as
748+
// git bash. In these situations the coder.exe is using the windows filepath
749+
// separator (\), but the shell wants the unix filepath separator (/).
750+
// Trying to determine if the shell is unix-like is difficult, so this function
751+
// takes the argument 'forceUnixPath' to force the filepath to be unix-like.
752+
//
753+
// On actual unix machines, this is **always** a noop. Even if a windows
754+
// path is provided.
755+
//
756+
// Passing a "false" for forceUnixPath will result in the filepath separator
757+
// untouched from the original input.
758+
// ---
759+
// This is a control flag, and that is ok. It is a control flag
760+
// based on the OS of the user. Making this a different file is excessive.
761+
// nolint:revive
762+
func sshConfigExecEscape(path string, forceUnixPath bool) (string, error) {
763+
if forceUnixPath {
764+
// This is a workaround for #7639, where the filepath separator is
765+
// incorrectly the Windows separator (\) instead of the unix separator (/).
766+
path = filepath.ToSlash(path)
767+
}
768+
731769
// This is unlikely to ever happen, but newlines are allowed on
732770
// certain filesystems, but cannot be used inside ssh config.
733771
if strings.ContainsAny(path, "\n") {

‎cli/configssh_internal_test.go

+79-8
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,11 @@ import (
1212
"github.com/stretchr/testify/require"
1313
)
1414

15+
func init() {
16+
// For golden files, always show the flag.
17+
hideForceUnixSlashes = false
18+
}
19+
1520
func Test_sshConfigSplitOnCoderSection(t *testing.T) {
1621
t.Parallel()
1722

@@ -140,14 +145,14 @@ func Test_sshConfigExecEscape(t *testing.T) {
140145
name string
141146
path string
142147
wantErr bool
143-
windows bool
144148
}{
145-
{"no spaces", "simple", false, true},
146-
{"spaces", "path with spaces", false, true},
147-
{"quotes", "path with \"quotes\"", false, false},
148-
{"backslashes", "path with \\backslashes", false, false},
149-
{"tabs", "path with \ttabs", false, false},
150-
{"newline fails", "path with \nnewline", true, false},
149+
{"windows path", `C:\Program Files\Coder\bin\coder.exe`, false},
150+
{"no spaces", "simple", false},
151+
{"spaces", "path with spaces", false},
152+
{"quotes", "path with \"quotes\"", false},
153+
{"backslashes", "path with \\backslashes", false},
154+
{"tabs", "path with \ttabs", false},
155+
{"newline fails", "path with \nnewline", true},
151156
}
152157
for _, tt := range tests {
153158
tt := tt
@@ -166,7 +171,7 @@ func Test_sshConfigExecEscape(t *testing.T) {
166171
err = os.WriteFile(bin, contents, 0o755) //nolint:gosec
167172
require.NoError(t, err)
168173

169-
escaped, err := sshConfigExecEscape(bin)
174+
escaped, err := sshConfigExecEscape(bin, false)
170175
if tt.wantErr {
171176
require.Error(t, err)
172177
return
@@ -181,6 +186,72 @@ func Test_sshConfigExecEscape(t *testing.T) {
181186
}
182187
}
183188

189+
func Test_sshConfigExecEscapeSeparatorForce(t *testing.T) {
190+
t.Parallel()
191+
192+
tests := []struct {
193+
name string
194+
path string
195+
// Behavior is different on Windows
196+
expWindowsPath string
197+
expOtherPath string
198+
forceUnix bool
199+
wantErr bool
200+
}{
201+
{
202+
name: "windows_keep_forward_slashes_with_spaces",
203+
// Has a space, expect quotes
204+
path: `C:\Program Files\Coder\bin\coder.exe`,
205+
expWindowsPath: `"C:\Program Files\Coder\bin\coder.exe"`,
206+
expOtherPath: `"C:\Program Files\Coder\bin\coder.exe"`,
207+
forceUnix: false,
208+
wantErr: false,
209+
},
210+
{
211+
name: "windows_keep_forward_slashes",
212+
path: `C:\ProgramFiles\Coder\bin\coder.exe`,
213+
expWindowsPath: `C:\ProgramFiles\Coder\bin\coder.exe`,
214+
expOtherPath: `C:\ProgramFiles\Coder\bin\coder.exe`,
215+
forceUnix: false,
216+
wantErr: false,
217+
},
218+
{
219+
name: "windows_force_unix_with_spaces",
220+
path: `C:\Program Files\Coder\bin\coder.exe`,
221+
expWindowsPath: `"C:/Program Files/Coder/bin/coder.exe"`,
222+
expOtherPath: `"C:\Program Files\Coder\bin\coder.exe"`,
223+
forceUnix: true,
224+
wantErr: false,
225+
},
226+
{
227+
name: "windows_force_unix",
228+
path: `C:\ProgramFiles\Coder\bin\coder.exe`,
229+
expWindowsPath: `C:/ProgramFiles/Coder/bin/coder.exe`,
230+
expOtherPath: `C:\ProgramFiles\Coder\bin\coder.exe`,
231+
forceUnix: true,
232+
wantErr: false,
233+
},
234+
}
235+
for _, tt := range tests {
236+
tt := tt
237+
t.Run(tt.name, func(t *testing.T) {
238+
t.Parallel()
239+
found, err := sshConfigExecEscape(tt.path, tt.forceUnix)
240+
if tt.wantErr {
241+
require.Error(t, err)
242+
return
243+
}
244+
require.NoError(t, err)
245+
if runtime.GOOS == "windows" {
246+
require.Equal(t, tt.expWindowsPath, found, "(Windows) expected path")
247+
} else {
248+
// this is a noop on non-windows!
249+
require.Equal(t, tt.expOtherPath, found, "(Non-Windows) expected path")
250+
}
251+
})
252+
}
253+
}
254+
184255
func Test_sshConfigOptions_addOption(t *testing.T) {
185256
t.Parallel()
186257
testCases := []struct {

‎cli/configssh_other.go

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
//go:build !windows
2+
3+
package cli
4+
5+
var hideForceUnixSlashes = true

‎cli/configssh_windows.go

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
//go:build windows
2+
3+
package cli
4+
5+
// Must be a var for unit tests to conform behavior
6+
var hideForceUnixSlashes = false

‎cli/testdata/coder_config-ssh_--help.golden

+6
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,12 @@ Add an SSH Host entry for your workspaces "ssh coder.workspace"
1515
-n, --dry-run bool, $CODER_SSH_DRY_RUN
1616
Perform a trial run with no changes made, showing a diff at the end.
1717

18+
--force-unix-filepaths bool, $CODER_CONFIGSSH_UNIX_FILEPATHS
19+
By default, 'config-ssh' uses the os path separator when writing the
20+
ssh config. This might be an issue in Windows machine that use a
21+
unix-like shell. This flag forces the use of unix file paths (the
22+
forward slash '/').
23+
1824
--ssh-config-file string, $CODER_SSH_CONFIG_FILE (default: ~/.ssh/config)
1925
Specifies the path to an SSH config.
2026

0 commit comments

Comments
 (0)