Warn if coder-cli is not in PATH on Windows (#357)

* Warn if coder-cli not in PATH on Windows

* Use safeexec to get windows path

* Delete GOARCH=386 from windows build

Author: deansheather

.github/workflows/release.yaml

@@ -22,8 +22,8 @@ jobs:
       - name: Upload windows
         uses: actions/upload-artifact@v2
         with:
-          name: coder-cli-windows-386
-          path: ./ci/bin/coder-cli-windows-386.zip
+          name: coder-cli-windows
+          path: ./ci/bin/coder-cli-windows.zip
   build_darwin:
     name: Build darwin binary
     runs-on: macos-latest
@@ -74,7 +74,7 @@ jobs:
           draft: true
           prerelease: false
       - name: Upload Linux Release
-        id: upload-linux-release-asset 
+        id: upload-linux-release-asset
         uses: actions/upload-release-asset@v1
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
@@ -84,7 +84,7 @@ jobs:
           asset_name: coder-cli-linux-amd64.tar.gz
           asset_content_type: application/tar+gzip
       - name: Upload MacOS Release
-        id: upload-macos-release-asset 
+        id: upload-macos-release-asset
         uses: actions/upload-release-asset@v1
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
@@ -94,12 +94,12 @@ jobs:
           asset_name: coder-cli-darwin-amd64.zip
           asset_content_type: application/zip
       - name: Upload Windows Release
-        id: upload-windows-release-asset 
+        id: upload-windows-release-asset
         uses: actions/upload-release-asset@v1
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
         with:
           upload_url: ${{ steps.create_release.outputs.upload_url }}
-          asset_path: coder-cli-windows-386/coder-cli-windows-386.zip
-          asset_name: coder-cli-windows-386.zip
+          asset_path: coder-cli-windows/coder-cli-windows.zip
+          asset_name: coder-cli-windows.zip
           asset_content_type: application/zip

Makefile

@@ -14,7 +14,7 @@ build/macos:
 	# requires darwin
 	CGO_ENABLED=1 GOOS=darwin GOARCH=amd64 ./ci/scripts/build.sh
 build/windows:
-	CGO_ENABLED=0 GOOS=windows GOARCH=386 ./ci/scripts/build.sh
+	CGO_ENABLED=0 GOOS=windows ./ci/scripts/build.sh
 build/linux:
 	CGO_ENABLED=0 GOOS=linux GOARCH=amd64 ./ci/scripts/build.sh
 

ci/scripts/build.sh

@@ -8,19 +8,26 @@ set -euo pipefail
 
 cd "$(git rev-parse --show-toplevel)/ci/scripts"
 
-tag=$(git describe --tags)
-
-echo "--- building coder-cli for $GOOS-$GOARCH"
-
-tmpdir=$(mktemp -d)
+tag="$(git describe --tags)"
+
+flavor="$GOOS"
+if [[ "$GOOS" == "windows" ]]; then
+	# GOARCH causes bugs with the safeexec package on Windows.
+	unset GOARCH
+else
+	flavor+="-$GOARCH"
+fi
+echo "--- building coder-cli for $flavor"
+
+tmpdir="$(mktemp -d)"
 go build -ldflags "-X cdr.dev/coder-cli/internal/version.Version=${tag}" -o "$tmpdir/coder" ../../cmd/coder
 
 cp ../gon.json $tmpdir/gon.json
 
 pushd "$tmpdir"
 case "$GOOS" in
 "windows")
-	artifact="coder-cli-$GOOS-$GOARCH.zip"
+	artifact="coder-cli-$GOOS.zip"
 	mv coder coder.exe
 	zip "$artifact" coder.exe
 	;;

go.mod

@@ -6,6 +6,7 @@ require (
 	cdr.dev/slog v1.4.1
 	cdr.dev/wsep v0.0.0-20200728013649-82316a09813f
 	github.com/briandowns/spinner v1.16.0
+	github.com/cli/safeexec v1.0.0
 	github.com/fatih/color v1.12.0
 	github.com/google/go-cmp v0.5.6
 	github.com/gorilla/websocket v1.4.2

go.sum

@@ -78,6 +78,8 @@ github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e h1:fY5BOSpyZCqRo5O
 github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e/go.mod h1:nSuG5e5PlCu98SY8svDHJxuZscDgtXS6KTTbou5AhLI=
 github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1 h1:q763qf9huN11kDQavWsoZXJNW3xEE4JJyHa5Q25/sd8=
 github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1/go.mod h1:Q3SI9o4m/ZMnBNeIyt5eFwwo7qiLfzFZmjNmxjkiQlU=
+github.com/cli/safeexec v1.0.0 h1:0VngyaIyqACHdcMNWfo6+KdUYnqEr2Sg+bSP1pdF+dI=
+github.com/cli/safeexec v1.0.0/go.mod h1:Z/D4tTN8Vs5gXYHDCbaM1S/anmEDnJb1iW0+EJ5zx3Q=
 github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw=
 github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGXZJjfX53e64911xZQV5JYwmTeXPW+k8Sc=
 github.com/cncf/udpa/go v0.0.0-20200629203442-efcf912fb354/go.mod h1:WmhPx2Nbnhtbo57+VJT5O0JRkEi1Wbu0z5j0R8u5Hbk=

internal/cmd/configssh.go

@@ -12,13 +12,13 @@ import (
 	"sort"
 	"strings"
 
-	"cdr.dev/coder-cli/pkg/clog"
-
+	"github.com/cli/safeexec"
 	"github.com/spf13/cobra"
 	"golang.org/x/xerrors"
 
 	"cdr.dev/coder-cli/coder-sdk"
 	"cdr.dev/coder-cli/internal/coderutil"
+	"cdr.dev/coder-cli/pkg/clog"
 )
 
 const sshStartToken = "# ------------START-CODER-ENTERPRISE-----------"
@@ -114,7 +114,7 @@ func configSSH(configpath *string, remove *bool) func(cmd *cobra.Command, _ []st
 			return xerrors.New("SSH is disabled or not available for any workspaces in your Coder deployment.")
 		}
 
-		binPath, err := os.Executable()
+		binPath, err := binPath()
 		if err != nil {
 			return xerrors.Errorf("Failed to get executable path: %w", err)
 		}
@@ -147,6 +147,53 @@ func configSSH(configpath *string, remove *bool) func(cmd *cobra.Command, _ []st
 	}
 }
 
+// binPath returns the path to the coder binary suitable for use in ssh
+// ProxyCommand.
+func binPath() (string, error) {
+	exePath, err := os.Executable()
+	if err != nil {
+		return "", xerrors.Errorf("get executable path: %w", err)
+	}
+
+	// On Windows, the coder-cli executable must be in $PATH for both Msys2/Git
+	// Bash and OpenSSH for Windows (used by Powershell and VS Code) to function
+	// correctly. Check if the current executable is in $PATH, and warn the user
+	// if it isn't.
+	if runtime.GOOS == "windows" {
+		binName := filepath.Base(exePath)
+
+		// We use safeexec instead of os/exec because os/exec returns paths in
+		// the current working directory, which we will run into very often when
+		// looking for our own path.
+		pathPath, err := safeexec.LookPath(binName)
+		if err != nil {
+			clog.LogWarn(
+				"The current executable is not in $PATH.",
+				"This may lead to problems connecting to your workspace via SSH.",
+				fmt.Sprintf("Please move %q to a location in your $PATH (such as System32) and run `%s config-ssh` again.", binName, binName),
+			)
+			// Return the exePath so SSH at least works outside of Msys2.
+			return exePath, nil
+		}
+
+		// Warn the user if the current executable is not the same as the one in
+		// $PATH.
+		if filepath.Clean(pathPath) != filepath.Clean(exePath) {
+			clog.LogWarn(
+				"The current executable path does not match the executable path found in $PATH.",
+				"This may lead to problems connecting to your workspace via SSH.",
+				fmt.Sprintf("\t Current executable path: %q", exePath),
+				fmt.Sprintf("\tExecutable path in $PATH: %q", pathPath),
+			)
+		}
+
+		return binName, nil
+	}
+
+	// On platforms other than Windows we can use the full path to the binary.
+	return exePath, nil
+}
+
 // removeOldConfig removes the old ssh configuration from the user's sshconfig.
 // Returns true if the config was modified.
 func removeOldConfig(config string) (string, bool) {
@@ -212,7 +259,7 @@ func makeSSHConfig(binPath, host, userName, workspaceName, privateKeyFilepath st
 		host := fmt.Sprintf(
 			`Host coder.%s
    HostName coder.%s
-   ProxyCommand %s tunnel %s 12213 stdio
+   ProxyCommand "%s" tunnel %s 12213 stdio
    StrictHostKeyChecking no
    ConnectTimeout=0
    IdentitiesOnly yes