chore: improve errors

Improve the reported errors so that consumers can understand why failure
occurs.

This is vital for credential lookup failures, which is impossible to debug
otherwise, for example permission error when using gcloud as credentials
helper as identified debugging an issue in testcontainers.

Eliminate wrapping errors with "error ..." as that results in messages
which are hard to understand in a multi wrap.

Add tests to cover credentials helper, ensuring that command handling
behaves correctly.

Modernise error checks to use errors.Is.

Leverage doc comment reference linking instead of backticks.

Use Auth fallback if credentials store reports not found.

Remove impossible decode length check and ensure decoded data is sized
correctly, eliminating need for \x00 trim.

Add github action to run test and lint.

Fix from base64 auth tests not running due to missing parameter.

Fix some comment typos.

.github/workflows/go.yaml

@@ -0,0 +1,39 @@
+# This workflow will build a golang project
+# For more information see: https://docs.github.com/en/actions/automating-builds-and-tests/building-and-testing-go
+
+name: CI
+
+on:
+  push:
+    branches: ["main"]
+  pull_request:
+    branches: ["main"]
+
+env:
+  GO_VERSION: "1.23"
+  GOLANGCI_VERSION: "v1.59.1"
+
+jobs:
+  lint-test-go:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check out code
+        uses: actions/checkout@v4
+
+      - uses: actions/setup-go@v5
+        with:
+          go-version: ${{ env.GO_VERSION }}
+
+      - name: Validate go mod tidy
+        run: |
+          go mod tidy
+          git --no-pager diff && [[ 0 -eq $(git status --porcelain | wc -l) ]]
+
+      - name: Go Lint
+        uses: golangci/golangci-lint-action@v6
+        with:
+          version: ${{ env.GOLANGCI_VERSION }}
+          args: --out-format=colored-line-number
+
+      - name: Test
+        run: go test -race ./...

auth.go

@@ -1,40 +1,43 @@
 package dockercfg
 
 import (
+	"bytes"
 	"encoding/base64"
 	"encoding/json"
 	"errors"
 	"fmt"
-	"os"
+	"io/fs"
 	"os/exec"
 	"runtime"
 	"strings"
 )
 
-// This is used by the docker CLI in casses where an oauth identity token is used.
-// In that case the username is stored litterally as `<token>`
-// When fetching the credentials we check for this value to determine if
+// This is used by the docker CLI in cases where an oauth identity token is used.
+// In that case the username is stored literally as `<token>`
+// When fetching the credentials we check for this value to determine if.
 const tokenUsername = "<token>"
 
 // GetRegistryCredentials gets registry credentials for the passed in registry host.
 //
-// This will use `LoadDefaultConfig` to read registry auth details from the config.
+// This will use [LoadDefaultConfig] to read registry auth details from the config.
 // If the config doesn't exist, it will attempt to load registry credentials using the default credential helper for the platform.
 func GetRegistryCredentials(hostname string) (string, string, error) {
 	cfg, err := LoadDefaultConfig()
 	if err != nil {
-		if !os.IsNotExist(err) {
-			return "", "", err
+		if !errors.Is(err, fs.ErrNotExist) {
+			return "", "", fmt.Errorf("load default config: %w", err)
 		}
+
 		return GetCredentialsFromHelper("", hostname)
 	}
+
 	return cfg.GetRegistryCredentials(hostname)
 }
 
 // ResolveRegistryHost can be used to transform a docker registry host name into what is used for the docker config/cred helpers
 //
 // This is useful for using with containerd authorizers.
-// Natrually this only transforms docker hub URLs.
+// Naturally this only transforms docker hub URLs.
 func ResolveRegistryHost(host string) string {
 	switch host {
 	case "index.docker.io", "docker.io", "https://index.docker.io/v1/", "registry-1.docker.io":
@@ -43,9 +46,9 @@ func ResolveRegistryHost(host string) string {
 	return host
 }
 
-// GetRegistryCredentials gets credentials, if any, for the provided hostname
+// GetRegistryCredentials gets credentials, if any, for the provided hostname.
 //
-// Hostnames should already be resolved using `ResolveRegistryAuth`
+// Hostnames should already be resolved using [ResolveRegistryHost].
 //
 // If the returned username string is empty, the password is an identity token.
 func (c *Config) GetRegistryCredentials(hostname string) (string, string, error) {
@@ -55,7 +58,14 @@ func (c *Config) GetRegistryCredentials(hostname string) (string, string, error)
 	}
 
 	if c.CredentialsStore != "" {
-		return GetCredentialsFromHelper(c.CredentialsStore, hostname)
+		username, password, err := GetCredentialsFromHelper(c.CredentialsStore, hostname)
+		if err != nil {
+			return "", "", fmt.Errorf("get credentials from store: %w", err)
+		}
+
+		if username != "" || password != "" {
+			return username, password, nil
+		}
 	}
 
 	auth, ok := c.AuthConfigs[hostname]
@@ -87,78 +97,96 @@ func DecodeBase64Auth(auth AuthConfig) (string, string, error) {
 	decoded := make([]byte, decLen)
 	n, err := base64.StdEncoding.Decode(decoded, []byte(auth.Auth))
 	if err != nil {
-		return "", "", fmt.Errorf("error decoding auth from file: %w", err)
+		return "", "", fmt.Errorf("decode auth: %w", err)
 	}
 
-	if n > decLen {
-		return "", "", fmt.Errorf("decoded value is longer than expected length, expected: %d, actual: %d", decLen, n)
-	}
+	decoded = decoded[:n]
 
-	split := strings.SplitN(string(decoded), ":", 2)
-	if len(split) != 2 {
-		return "", "", errors.New("invalid auth string")
+	const sep = ":"
+	user, pass, found := strings.Cut(string(decoded), sep)
+	if !found {
+		return "", "", fmt.Errorf("invalid auth: missing %q separator", sep)
 	}
 
-	return split[0], strings.Trim(split[1], "\x00"), nil
+	return user, pass, nil
 }
 
-// Errors from credential helpers
+// Errors from credential helpers.
 var (
 	ErrCredentialsNotFound         = errors.New("credentials not found in native keychain")
 	ErrCredentialsMissingServerURL = errors.New("no credentials server URL")
 )
 
+//nolint:gochecknoglobals // These are used to mock exec in tests.
+var (
+	// execLookPath is a variable that can be used to mock exec.LookPath in tests.
+	execLookPath = exec.LookPath
+	// execCommand is a variable that can be used to mock exec.Command in tests.
+	execCommand = exec.Command
+)
+
 // GetCredentialsFromHelper attempts to lookup credentials from the passed in docker credential helper.
 //
-// The credential helpoer should just be the suffix name (no "docker-credential-").
+// The credential helper should just be the suffix name (no "docker-credential-").
 // If the passed in helper program is empty this will look up the default helper for the platform.
 //
 // If the credentials are not found, no error is returned, only empty credentials.
 //
-// Hostnames should already be resolved using `ResolveRegistryAuth`
+// Hostnames should already be resolved using [ResolveRegistryHost]
 //
 // If the username string is empty, the password string is an identity token.
 func GetCredentialsFromHelper(helper, hostname string) (string, string, error) {
 	if helper == "" {
-		helper = getCredentialHelper()
-	}
-	if helper == "" {
-		return "", "", nil
+		helper, helperErr := getCredentialHelper()
+		if helperErr != nil {
+			return "", "", fmt.Errorf("get credential helper: %w", helperErr)
+		}
+
+		if helper == "" {
+			return "", "", nil
+		}
 	}
 
-	p, err := exec.LookPath("docker-credential-" + helper)
+	helper = "docker-credential-" + helper
+	p, err := execLookPath(helper)
 	if err != nil {
+		if !errors.Is(err, exec.ErrNotFound) {
+			return "", "", fmt.Errorf("look up %q: %w", helper, err)
+		}
+
 		return "", "", nil
 	}
 
-	cmd := exec.Command(p, "get")
+	var outBuf, errBuf bytes.Buffer
+	cmd := execCommand(p, "get")
 	cmd.Stdin = strings.NewReader(hostname)
+	cmd.Stdout = &outBuf
+	cmd.Stderr = &errBuf
 
-	b, err := cmd.Output()
-	if err != nil {
-		s := strings.TrimSpace(string(b))
-
-		switch s {
+	if err = cmd.Run(); err != nil {
+		out := strings.TrimSpace(outBuf.String())
+		switch out {
 		case ErrCredentialsNotFound.Error():
 			return "", "", nil
 		case ErrCredentialsMissingServerURL.Error():
-			return "", "", errors.New(s)
+			return "", "", ErrCredentialsMissingServerURL
 		default:
+			return "", "", fmt.Errorf("execute %q stdout: %q stderr: %q: %w",
+				helper, out, strings.TrimSpace(errBuf.String()), err,
+			)
 		}
-
-		return "", "", err
 	}
 
 	var creds struct {
-		Username string
-		Secret   string
+		Username string `json:"Username"`
+		Secret   string `json:"Secret"`
 	}
 
-	if err := json.Unmarshal(b, &creds); err != nil {
-		return "", "", err
+	if err = json.Unmarshal(outBuf.Bytes(), &creds); err != nil {
+		return "", "", fmt.Errorf("unmarshal credentials from: %q: %w", helper, err)
 	}
 
-	// When tokenUsername is used, the output is an identity token and the username is garbage
+	// When tokenUsername is used, the output is an identity token and the username is garbage.
 	if creds.Username == tokenUsername {
 		creds.Username = ""
 	}
@@ -167,18 +195,21 @@ func GetCredentialsFromHelper(helper, hostname string) (string, string, error) {
 }
 
 // getCredentialHelper gets the default credential helper name for the current platform.
-func getCredentialHelper() string {
+func getCredentialHelper() (string, error) {
 	switch runtime.GOOS {
 	case "linux":
-		if _, err := exec.LookPath("pass"); err == nil {
-			return "pass"
+		if _, err := exec.LookPath("pass"); err != nil {
+			if errors.Is(err, exec.ErrNotFound) {
+				return "secretservice", nil
+			}
+			return "", fmt.Errorf(`look up "pass": %w`, err)
 		}
-		return "secretservice"
+		return "pass", nil
 	case "darwin":
-		return "osxkeychain"
+		return "osxkeychain", nil
 	case "windows":
-		return "wincred"
+		return "wincred", nil
 	default:
-		return ""
+		return "", nil
 	}
 }