feat(plugins): add windows support

.github/workflows/main.yml

@@ -63,6 +63,34 @@ jobs:
           name: unit-tests-artifact
           path: ./coverage-unit-tests.out
 
+  windows-tests:
+    name: Plugin Tests (Windows)
+    runs-on: windows-latest
+    steps:
+      - name: Check out the code
+        uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+
+      - name: Set up Go
+        uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0
+        with:
+          go-version-file: go.mod
+          check-latest: true
+
+      - name: Run plugin library tests
+        run: go test -count=1 ./internal/plugins/...
+
+      - name: Run plugin command tests
+        if: ${{ !cancelled() }}
+        run: go test -count=1 -run 'Plugin|Approvals|Commands|InterruptGuard|OwnedFlags|NamespaceJSONHelp|RegistryNextAction' ./internal/cli
+
+      - name: Run plugin library tests with the test registry key
+        if: ${{ !cancelled() }}
+        run: go test -count=1 -tags plugintest ./internal/plugins/...
+
+      - name: Run plugin command tests with the test registry key
+        if: ${{ !cancelled() }}
+        run: go test -count=1 -tags plugintest -run 'Plugin|Approvals|Commands|InterruptGuard|OwnedFlags|NamespaceJSONHelp|RegistryNextAction' ./internal/cli
+
   integration-tests:
     name: Integration Tests
     runs-on: ubuntu-latest

internal/cli/plugin_discovery.go

@@ -282,6 +282,8 @@ func writePluginHint(path string, shown time.Time) {
 	_, writeErr := temporary.Write(data)
 	closeErr := temporary.Close()
 
+	// This is os.Rename and not plugins.Rename: the hint is best effort, and the
+	// retry could hold up a command by over a second for a write that may fail.
 	if writeErr != nil || closeErr != nil || os.Rename(temporary.Name(), path) != nil {
 		_ = os.Remove(temporary.Name())
 	}

internal/cli/plugin_discovery_test.go

@@ -10,7 +10,6 @@ import (
 	"runtime"
 	"strconv"
 	"strings"
-	"syscall"
 	"testing"
 	"time"
 
@@ -897,29 +896,6 @@ func TestReadPluginHint(t *testing.T) {
 		assert.False(t, ok)
 	})
 
-	t.Run("a named pipe does not hold the command up", func(t *testing.T) {
-		if runtime.GOOS == "windows" {
-			t.Skip("no named pipes here")
-		}
-
-		pipe := filepath.Join(dir, "pipe")
-		require.NoError(t, syscall.Mkfifo(pipe, 0o600))
-
-		done := make(chan bool, 1)
-
-		go func() {
-			_, ok := readPluginHint(pipe)
-			done <- ok
-		}()
-
-		select {
-		case ok := <-done:
-			assert.False(t, ok)
-		case <-time.After(10 * time.Second):
-			t.Fatal("reading the hint file blocked on a named pipe")
-		}
-	})
-
 	t.Run("a symbolic link is not followed", func(t *testing.T) {
 		if runtime.GOOS == "windows" {
 			t.Skip("no symbolic links without a privilege")

internal/cli/plugin_discovery_unix_test.go

@@ -0,0 +1,32 @@
+//go:build !windows
+
+package cli
+
+import (
+	"path/filepath"
+	"syscall"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestReadPluginHint_NamedPipe(t *testing.T) {
+	pipe := filepath.Join(t.TempDir(), "pipe")
+	require.NoError(t, syscall.Mkfifo(pipe, 0o600))
+
+	done := make(chan bool, 1)
+
+	go func() {
+		_, ok := readPluginHint(pipe)
+		done <- ok
+	}()
+
+	select {
+	case ok := <-done:
+		assert.False(t, ok)
+	case <-time.After(10 * time.Second):
+		t.Fatal("reading the hint file blocked on a named pipe")
+	}
+}

internal/cli/plugin_run_test.go

@@ -20,7 +20,6 @@ import (
 	"strconv"
 	"strings"
 	"sync"
-	"syscall"
 	"testing"
 	"time"
 
@@ -68,7 +67,7 @@ type fixtureReportDoc struct {
 // --code=, --path= and --marker=. Everything else is only reported, which is how
 // a test sees the arguments that the CLI passed on.
 func runPluginFixture(args []string) (code int, ok bool) {
-	if strings.TrimSuffix(filepath.Base(args[0]), ".exe") != pluginTestBin {
+	if filepath.Base(args[0]) != registrytest.BinName(plugins.CurrentPlatform(), pluginTestBin) {
 		return 0, false
 	}
 
@@ -372,11 +371,12 @@ func (r *runEnv) releaseFixture(version string, commands []plugins.Command) regi
 	data, err := json.Marshal(manifest)
 	require.NoError(r.t, err)
 
-	archive := fixtureArchive(r.t, pluginTestBin, data)
+	bin := registrytest.BinName(r.platform, pluginTestBin)
+	archive := fixtureArchive(r.t, bin, data)
 	path := "/auth0/auth0-cli-plugins/releases/download/v" + version + "/" + pluginTestBin + ".zip"
 
 	record.Artifacts = []registry.Artifact{
-		registrytest.NewArtifact(r.platform, "https://github.com"+path, registry.ArtifactFormatZip, pluginTestBin, archive),
+		registrytest.NewArtifact(r.platform, "https://github.com"+path, registry.ArtifactFormatZip, bin, archive),
 	}
 
 	r.server.Set(path, archive)
@@ -495,16 +495,6 @@ func waitForMarker(t *testing.T, path string) int {
 	return pid
 }
 
-// processAlive reports whether a process with the ID exists.
-func processAlive(pid int) bool {
-	process, err := os.FindProcess(pid)
-	if err != nil {
-		return false
-	}
-
-	return process.Signal(syscall.Signal(0)) == nil
-}
-
 func TestRegisterPluginCommands_Gate(t *testing.T) {
 	e := newPluginEnv(t)
 	e.publish(e.index(e.release("1.0.0")))
@@ -1005,6 +995,16 @@ func TestPluginRun_Environment(t *testing.T) {
 	t.Setenv("LANG", "en_US.UTF-8")
 
 	allowed := func(name string) bool {
+		// Windows names are not case sensitive, and a plugin gets the variables that
+		// it needs to find its way around the system.
+		if runtime.GOOS == "windows" {
+			for _, system := range []string{"SystemRoot", "TEMP", "TMP", "USERPROFILE", "APPDATA", "LOCALAPPDATA", "PATHEXT", "ComSpec"} {
+				if strings.EqualFold(name, system) {
+					return true
+				}
+			}
+		}
+
 		switch name {
 		case "PATH", "HOME", "LANG", "LC_ALL", "LC_CTYPE", "TERM", "TMPDIR",
 			"HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY", "http_proxy", "https_proxy", "no_proxy",
@@ -1627,9 +1627,7 @@ func TestPluginRun_Results(t *testing.T) {
 		assert.Less(t, time.Since(started), 30*time.Second)
 		assert.JSONEq(t, `{"pluginId":"auth0/checkmate","version":"1.0.0"}`, string(buildErrorEnvelope(err).Error.Details))
 
-		if runtime.GOOS != "windows" {
-			assert.False(t, processAlive(waitForMarker(t, marker)), "the plugin must be gone")
-		}
+		assert.False(t, processAlive(waitForMarker(t, marker)), "the plugin must be gone")
 	})
 
 	t.Run("the timeout comes from the registry when the command has none", func(t *testing.T) {
@@ -1666,10 +1664,6 @@ func TestPluginRun_Results(t *testing.T) {
 }
 
 func TestPluginRun_Interrupt(t *testing.T) {
-	if runtime.GOOS == "windows" {
-		t.Skip("the process tree is stopped differently on Windows")
-	}
-
 	r := newRunEnv(t, fixtureCommands())
 	r.approve()
 
@@ -1701,35 +1695,6 @@ func TestPluginRun_Interrupt(t *testing.T) {
 	assert.False(t, processAlive(pid), "the plugin must be gone")
 }
 
-func TestPluginRun_Hangup(t *testing.T) {
-	if runtime.GOOS == "windows" {
-		t.Skip("the process tree is stopped differently on Windows")
-	}
-
-	r := newRunEnv(t, fixtureCommands())
-	r.approve()
-
-	marker := filepath.Join(t.TempDir(), "pid")
-	done := make(chan error, 1)
-
-	go func() { done <- r.exec("checkmate", "--mode=sleep", "--marker="+marker) }()
-
-	pid := waitForMarker(t, marker)
-
-	// A terminal that is closed sends SIGHUP to the CLI. The run has taken the
-	// signal by now, as the plugin only starts after that.
-	require.NoError(t, syscall.Kill(os.Getpid(), syscall.SIGHUP))
-
-	err := <-done
-
-	require.Error(t, err)
-
-	class, reason := reasonOf(err)
-	assert.Equal(t, "plugin", class)
-	assert.Equal(t, "plugin_interrupted", reason)
-	assert.False(t, processAlive(pid), "the plugin must be gone")
-}
-
 func TestPluginRun_AmbiguousSubcommand(t *testing.T) {
 	// The root command declares the boolean flag mode, so that cobra does not take
 	// the word after it as its value.
@@ -1945,16 +1910,35 @@ func TestPluginEnvironment_LoopbackIsNotProxied(t *testing.T) {
 	r.approve()
 
 	t.Setenv("NO_PROXY", "internal.example.com")
-	t.Setenv("no_proxy", "")
 	t.Setenv("HTTPS_PROXY", "http://proxy.example.com:3128")
 
+	// On Windows this is the same variable as NO_PROXY, which would be cleared.
+	if runtime.GOOS != "windows" {
+		t.Setenv("no_proxy", "")
+	}
+
 	require.NoError(t, r.exec("checkmate"))
 
 	env := r.report(t).Env
 
-	assert.Equal(t, "internal.example.com,127.0.0.1,localhost,::1", env["NO_PROXY"])
-	assert.Equal(t, "127.0.0.1,localhost,::1", env["no_proxy"])
-	assert.Equal(t, "http://proxy.example.com:3128", env["HTTPS_PROXY"], "the proxy of the user is kept")
+	// On Windows a name has one spelling in the environment of the plugin, and it is
+	// not necessarily the one that was set.
+	lookup := func(name string) string {
+		for key, value := range env {
+			if key == name || (runtime.GOOS == "windows" && strings.EqualFold(key, name)) {
+				return value
+			}
+		}
+
+		return ""
+	}
+
+	assert.Equal(t, "internal.example.com,127.0.0.1,localhost,::1", lookup("NO_PROXY"))
+	assert.Equal(t, "http://proxy.example.com:3128", lookup("HTTPS_PROXY"), "the proxy of the user is kept")
+
+	if runtime.GOOS != "windows" {
+		assert.Equal(t, "127.0.0.1,localhost,::1", env["no_proxy"])
+	}
 }
 
 func TestRenderNamespaceJSONHelp_LeavesPluginCommandsAlone(t *testing.T) {

internal/cli/plugin_run_unix_test.go

@@ -0,0 +1,48 @@
+//go:build !windows
+
+package cli
+
+import (
+	"os"
+	"path/filepath"
+	"syscall"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// processAlive reports whether a process with the ID exists.
+func processAlive(pid int) bool {
+	process, err := os.FindProcess(pid)
+	if err != nil {
+		return false
+	}
+
+	return process.Signal(syscall.Signal(0)) == nil
+}
+
+func TestPluginRun_Hangup(t *testing.T) {
+	r := newRunEnv(t, fixtureCommands())
+	r.approve()
+
+	marker := filepath.Join(t.TempDir(), "pid")
+	done := make(chan error, 1)
+
+	go func() { done <- r.exec("checkmate", "--mode=sleep", "--marker="+marker) }()
+
+	pid := waitForMarker(t, marker)
+
+	// A terminal that is closed sends SIGHUP to the CLI. The run has taken the
+	// signal by now, as the plugin only starts after that.
+	require.NoError(t, syscall.Kill(os.Getpid(), syscall.SIGHUP))
+
+	err := <-done
+
+	require.Error(t, err)
+
+	class, reason := reasonOf(err)
+	assert.Equal(t, "plugin", class)
+	assert.Equal(t, "plugin_interrupted", reason)
+	assert.False(t, processAlive(pid), "the plugin must be gone")
+}

internal/cli/plugin_run_windows_test.go

@@ -0,0 +1,29 @@
+package cli
+
+import (
+	"golang.org/x/sys/windows"
+)
+
+// stillActive is the exit code that GetExitCodeProcess reports for a process
+// that has not ended.
+const stillActive = 259
+
+// processAlive reports whether a process with the ID is still running. A process
+// that has ended can stay in the table while something holds a handle to it, so
+// the exit code decides, not whether the process can be opened.
+func processAlive(pid int) bool {
+	handle, err := windows.OpenProcess(windows.PROCESS_QUERY_LIMITED_INFORMATION, false, uint32(pid))
+	if err != nil {
+		return false
+	}
+
+	defer func() { _ = windows.CloseHandle(handle) }()
+
+	var code uint32
+
+	if err := windows.GetExitCodeProcess(handle, &code); err != nil {
+		return false
+	}
+
+	return code == stillActive
+}