cli/config/credentials: refactor DetectDefaultStore and add tests

Refactor the DetectDefaultStore to allow testing it cross-platform, and
without the actual helpers installed.

This also makes a small change in the logic for detecting the preferred
helper. Previously, it only detected the "helper" binary ("pass"), but
would fall back to using plain-text if the pass credentials-helper was
not installed.

With this patch, it falls back to the platform default (secretservice),
before falling back to using no credentials helper (and storing unencrypted).

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

cli/config/credentials/default_store.go

@@ -2,21 +2,70 @@ package credentials
 
 import "os/exec"
 
-// DetectDefaultStore return the default credentials store for the platform if
-// no user-defined store is passed, and the store executable is available.
-func DetectDefaultStore(store string) string {
-	if store != "" {
+// DetectDefaultStore returns the credentials store to use if no user-defined
+// custom helper is passed.
+//
+// Some platforms define a preferred helper, in which case it attempts to look
+// up the helper binary before falling back to the platform's default.
+//
+// If no user-defined helper is passed, and no helper is found, it returns an
+// empty string, which means credentials are stored unencrypted in the CLI's
+// config-file without the use of a credentials store.
+func DetectDefaultStore(customStore string) string {
+	if customStore != "" {
 		// use user-defined
-		return store
+		return customStore
+	}
+	if preferred := findPreferredHelper(); preferred != "" {
+		return preferred
+	}
+	if defaultHelper == "" {
+		return ""
+	}
+	if _, err := exec.LookPath(remoteCredentialsPrefix + defaultHelper); err != nil {
+		return ""
+	}
+	return defaultHelper
 }
 
-	platformDefault := defaultCredentialsStore()
-	if platformDefault == "" {
+// overridePreferred is used to override the preferred helper in tests.
+var overridePreferred string
+
+// findPreferredHelper detects whether the preferred credentials-store and
+// its helper binaries are installed. It returns the name of the preferred
+// store if found, otherwise returns an empty string to fall back to resolving
+// the default helper.
+//
+// Note that the logic below is currently specific to detection needed for the
+// "pass" credentials-helper on Linux (which is the only platform with a preferred
+// helper). It is put in a non-platform specific file to allow running tests
+// on other platforms.
+func findPreferredHelper() string {
+	preferred := preferredHelper
+	if overridePreferred != "" {
+		preferred = overridePreferred
+	}
+	if preferred == "" {
 		return ""
 	}
 
-	if _, err := exec.LookPath(remoteCredentialsPrefix + platformDefault); err != nil {
+	// Note that the logic below is specific to detection needed for the
+	// "pass" credentials-helper on Linux (which is the only platform with
+	// a "preferred" and "default" helper. This logic may change if a similar
+	// order of preference is needed on other platforms.
+
+	// If we don't have the preferred helper installed, there's no need
+	// to check if its dependencies are installed, instead, try to
+	// use the default credentials-helper for this platform (if installed).
+	if _, err := exec.LookPath(remoteCredentialsPrefix + preferred); err != nil {
 		return ""
 	}
-	return platformDefault
+
+	// Detect if the helper binary is present as well. This is needed for
+	// the "pass" credentials helper, which uses this binary.
+	if _, err := exec.LookPath(preferred); err != nil {
+		return ""
+	}
+
+	return preferred
 }

cli/config/credentials/default_store_darwin.go

@@ -1,5 +1,6 @@
 package credentials
 
-func defaultCredentialsStore() string {
-	return "osxkeychain"
-}
+const (
+	preferredHelper = ""
+	defaultHelper   = "osxkeychain"
+)

cli/config/credentials/default_store_linux.go

@@ -1,13 +1,6 @@
 package credentials
 
-import (
-	"os/exec"
+const (
+	preferredHelper = "pass"
+	defaultHelper   = "secretservice"
 )
-
-func defaultCredentialsStore() string {
-	if _, err := exec.LookPath("pass"); err == nil {
-		return "pass"
-	}
-
-	return "secretservice"
-}

cli/config/credentials/default_store_test.go

@@ -0,0 +1,74 @@
+package credentials
+
+import (
+	"os"
+	"path"
+	"testing"
+
+	"gotest.tools/v3/assert"
+)
+
+func TestDetectDefaultStore(t *testing.T) {
+	tmpDir := t.TempDir()
+	t.Setenv("PATH", tmpDir)
+
+	t.Run("none available", func(t *testing.T) {
+		const expected = ""
+		assert.Equal(t, expected, DetectDefaultStore(""))
+	})
+	t.Run("custom helper", func(t *testing.T) {
+		const expected = "my-custom-helper"
+		assert.Equal(t, expected, DetectDefaultStore(expected))
+
+		// Custom helper should be used even if the actual helper exists
+		createFakeHelper(t, path.Join(tmpDir, remoteCredentialsPrefix+defaultHelper))
+		assert.Equal(t, expected, DetectDefaultStore(expected))
+	})
+	t.Run("default", func(t *testing.T) {
+		createFakeHelper(t, path.Join(tmpDir, remoteCredentialsPrefix+defaultHelper))
+		expected := defaultHelper
+		assert.Equal(t, expected, DetectDefaultStore(""))
+	})
+
+	// On Linux, the "pass" credentials helper requires both a "pass" binary
+	// to be present and a "docker-credentials-pass" credentials helper to
+	// be installed.
+	t.Run("preferred helper", func(t *testing.T) {
+		// Create the default helper as we need it for the fallback.
+		createFakeHelper(t, path.Join(tmpDir, remoteCredentialsPrefix+defaultHelper))
+
+		const testPreferredHelper = "preferred"
+		overridePreferred = testPreferredHelper
+
+		// Use preferred helper if both binaries exist.
+		t.Run("success", func(t *testing.T) {
+			createFakeHelper(t, path.Join(tmpDir, testPreferredHelper))
+			createFakeHelper(t, path.Join(tmpDir, remoteCredentialsPrefix+testPreferredHelper))
+			expected := testPreferredHelper
+			assert.Equal(t, expected, DetectDefaultStore(""))
+		})
+
+		// Fall back to the default helper if the preferred credentials-helper isn't installed.
+		t.Run("not installed", func(t *testing.T) {
+			createFakeHelper(t, path.Join(tmpDir, remoteCredentialsPrefix+testPreferredHelper))
+			expected := defaultHelper
+			assert.Equal(t, expected, DetectDefaultStore(""))
+		})
+
+		// Similarly, fall back to the default helper if the preferred credentials-helper
+		// is installed, but the helper binary isn't found.
+		t.Run("missing helper", func(t *testing.T) {
+			createFakeHelper(t, path.Join(tmpDir, testPreferredHelper))
+			expected := defaultHelper
+			assert.Equal(t, expected, DetectDefaultStore(""))
+		})
+	})
+}
+
+func createFakeHelper(t *testing.T, fileName string) {
+	t.Helper()
+	assert.NilError(t, os.WriteFile(fileName, []byte("I'm a credentials-helper executable (really!)"), 0o700))
+	t.Cleanup(func() {
+		assert.NilError(t, os.RemoveAll(fileName))
+	})
+}

cli/config/credentials/default_store_unsupported.go

@@ -2,6 +2,7 @@
 
 package credentials
 
-func defaultCredentialsStore() string {
-	return ""
-}
+const (
+	preferredHelper = ""
+	defaultHelper   = ""
+)

cli/config/credentials/default_store_windows.go

@@ -1,5 +1,6 @@
 package credentials
 
-func defaultCredentialsStore() string {
-	return "wincred"
-}
+const (
+	preferredHelper = ""
+	defaultHelper   = "wincred"
+)