fixup test on Windows

spikecurtis · spikecurtis · commit caee378e01d3 · 2025-06-06T14:34:55.000+04:00
diff --git a/cli/configssh_internal_test.go b/cli/configssh_internal_test.go
@@ -193,16 +193,17 @@ func Test_sshConfigMatchExecEscape(t *testing.T) {
 	t.Parallel()
 
 	tests := []struct {
-		name    string
-		path    string
-		wantErr bool
+		name           string
+		path           string
+		wantErrOther   bool
+		wantErrWindows bool
 	}{
-		{"no spaces", "simple", false},
-		{"spaces", "path with spaces", false},
-		{"quotes fails", "path with \"quotes\"", true},
-		{"backslashes", "path with \\backslashes", false},
-		{"tabs", "path with \ttabs", false},
-		{"newline fails", "path with \nnewline", true},
+		{"no spaces", "simple", false, false},
+		{"spaces", "path with spaces", false, false},
+		{"quotes", "path with \"quotes\"", false, true},
+		{"backslashes", "path with\\backslashes", false, false},
+		{"tabs", "path with \ttabs", false, true},
+		{"newline fails", "path with \nnewline", true, true},
 	}
 	// nolint:paralleltest // Fixes a flake
 	for _, tt := range tests {
@@ -214,24 +215,26 @@ func Test_sshConfigMatchExecEscape(t *testing.T) {
 			if runtime.GOOS == "windows" {
 				cmd = "cmd.exe"
 				arg = "/c"
-				contents = []byte("echo yay\n")
+				contents = []byte("@echo yay\n")
 			}
 
 			dir := filepath.Join(t.TempDir(), tt.path)
-			err := os.MkdirAll(dir, 0o755)
-			require.NoError(t, err)
 			bin := filepath.Join(dir, "coder.bat") // Windows will treat it as batch, Linux doesn't care
-
-			err = os.WriteFile(bin, contents, 0o755) //nolint:gosec
-			require.NoError(t, err)
-
 			escaped, err := sshConfigMatchExecEscape(bin)
-			if tt.wantErr {
+			if (runtime.GOOS == "windows" && tt.wantErrWindows) || (runtime.GOOS != "windows" && tt.wantErrOther) {
 				require.Error(t, err)
 				return
 			}
 			require.NoError(t, err)
 
+			err = os.MkdirAll(dir, 0o755)
+			require.NoError(t, err)
+
+			err = os.WriteFile(bin, contents, 0o755) //nolint:gosec
+			require.NoError(t, err)
+
+			// OpenSSH processes %% escape sequences into %
+			escaped = strings.ReplaceAll(escaped, "%%", "%")
 			b, err := exec.Command(cmd, arg, escaped).CombinedOutput() //nolint:gosec
 			require.NoError(t, err)
 			got := strings.TrimSpace(string(b))
diff --git a/cli/configssh_windows.go b/cli/configssh_windows.go
@@ -34,25 +34,24 @@ var hideForceUnixSlashes = false
 //
 // Other notes:
 //   - @ in `@cmd.exe` suppresses echoing it, so you don't get this command printed
-//   - without another invocation of cmd.exe (e.g. `do @cmd.exe /c %%aC:\Program Files\Coder\bin\coder.exe%%a`) then
-//     the double-quote gets interpreted as part of the path and you get: '"C:\Program' is not recognized. Constructing
-//     the string and then passing it to another instance of cmd.exe does this trick here.
+//   - we need another invocation of cmd.exe (e.g. `do @cmd.exe /c %%aC:\Program Files\Coder\bin\coder.exe%%a`). Without
+//     it the double-quote gets interpreted as part of the path, and you get: '"C:\Program' is not recognized.
+//     Constructing the string and then passing it to another instance of cmd.exe does this trick here.
 //   - OpenSSH passes the `Match exec` command to cmd.exe regardless of whether the user has a unix-like shell like
 //     git bash, so we don't have a `forceUnixPath` option like for the ProxyCommand which does respect the user's
-//     configured shell.
+//     configured shell on Windows.
 func sshConfigMatchExecEscape(path string) (string, error) {
 	// This is unlikely to ever happen, but newlines are allowed on
 	// certain filesystems, but cannot be used inside ssh config.
 	if strings.ContainsAny(path, "\n") {
 		return "", xerrors.Errorf("invalid path: %s", path)
 	}
-	// Windows does not allow double-quotes in paths. If we get one it is an error.
-	if strings.Contains(path, `"`) {
-		return "", xerrors.Errorf("path must not contain quotes: %q", path)
+	// Windows does not allow double-quotes or tabs in paths. If we get one it is an error.
+	if strings.ContainsAny(path, "\"\t") {
+		return "", xerrors.Errorf("path must not contain quotes or tabs: %q", path)
 	}
-	// A space or a tab requires quoting, but tabs must not be escaped
-	// (\t) since OpenSSH interprets it as a literal \t, not a tab.
-	if strings.ContainsAny(path, " \t") {
+
+	if strings.ContainsAny(path, " ") {
 		// c.f. function comment for how this works.
 		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.
 	}
