Ignore DOCKER_AUTH_CONFIG without opt-in on Gitlab CI

cli/command/registry/login.go

@@ -111,7 +111,9 @@ func runLogin(ctx context.Context, dockerCLI command.Cli, opts loginOptions) err
 		return err
 	}
 
-	maybePrintEnvAuthWarning(dockerCLI)
+	if dockerCLI.ConfigFile().PreferDockerAuthConfig() {
+		printEnvAuthWarning(dockerCLI)
+	}
 
 	var (
 		serverAddress string

cli/command/registry/logout.go

@@ -36,7 +36,9 @@ func NewLogoutCommand(dockerCli command.Cli) *cobra.Command {
 }
 
 func runLogout(ctx context.Context, dockerCLI command.Cli, serverAddress string) error {
-	maybePrintEnvAuthWarning(dockerCLI)
+	if dockerCLI.ConfigFile().PreferDockerAuthConfig() {
+		printEnvAuthWarning(dockerCLI)
+	}
 
 	var isDefaultRegistry bool
 

cli/command/registry/warning.go

@@ -1,18 +1,14 @@
 package registry
 
 import (
-	"os"
-
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/cli/config/configfile"
 	"github.com/docker/cli/internal/tui"
 )
 
-// maybePrintEnvAuthWarning if the `DOCKER_AUTH_CONFIG` environment variable is
+// printEnvAuthWarning if the `DOCKER_AUTH_CONFIG` environment variable is
 // set this function will output a warning to stdErr
-func maybePrintEnvAuthWarning(out command.Streams) {
-	if os.Getenv(configfile.DockerEnvConfigKey) != "" {
-		tui.NewOutput(out.Err()).
-			PrintWarning("%[1]s is set and takes precedence.\nUnset %[1]s to restore the CLI auth behaviour.\n", configfile.DockerEnvConfigKey)
-	}
+func printEnvAuthWarning(out command.Streams) {
+	tui.NewOutput(out.Err()).
+		PrintWarning("%[1]s is set and takes precedence.\nUnset %[1]s to restore the CLI auth behaviour.\n", configfile.DockerEnvConfigKey)
 }

cli/config/configfile/file.go

@@ -287,6 +287,39 @@ func decodeAuth(authStr string) (string, string, error) {
 	return userName, strings.Trim(password, "\x00"), nil
 }
 
+// PreferDockerAuthConfig returns true if the memory store containing auth
+// config read from DOCKER_AUTH_CONFIG should be preferred over the config file.
+func (configFile *ConfigFile) PreferDockerAuthConfig() bool {
+	order := configFile.Features["auth_config_order"]
+	if v := os.Getenv("DOCKER_AUTH_ORDER"); v != "" {
+		order = v
+	}
+
+	order = strings.TrimSpace(order)
+	if order == "" {
+		// Temporary fix for clashing with GitLab CI
+		// Don't enable by default even if DOCKER_AUTH_CONFIG is set
+		// https://github.com/docker/cli/issues/6156
+		if os.Getenv("GITLAB_CI") == "true" {
+			return false
+		}
+		return true
+	}
+
+	parts := strings.Split(order, ",")
+	if len(parts) == 2 {
+		if parts[0] == "env" && parts[1] == "config" {
+			return true
+		}
+		if parts[0] == "config" && parts[1] == "env" {
+			return false
+		}
+	}
+
+	_, _ = fmt.Fprintln(os.Stderr, "Malformed DOCKER_AUTH_ORDER")
+	return true
+}
+
 // GetCredentialsStore returns a new credentials store from the settings in the
 // configuration file
 func (configFile *ConfigFile) GetCredentialsStore(registryHostname string) credentials.Store {
@@ -307,12 +340,17 @@ func (configFile *ConfigFile) GetCredentialsStore(registryHostname string) crede
 		return store
 	}
 
-	// use DOCKER_AUTH_CONFIG if set
-	// it uses the native or file store as a fallback to fetch and store credentials
-	envStore, err := memorystore.New(
+	opts := []memorystore.Options{
 		memorystore.WithAuthConfig(authConfig),
 		memorystore.WithFallbackStore(store),
-	)
+	}
+	if !configFile.PreferDockerAuthConfig() {
+		opts = append(opts, memorystore.WithPreferFallback())
+	}
+
+	// use DOCKER_AUTH_CONFIG if set
+	// it uses the native or file store as a fallback to fetch and store credentials
+	envStore, err := memorystore.New(opts...)
 	if err != nil {
 		_, _ = fmt.Fprintln(os.Stderr, "Failed to create credential store from DOCKER_AUTH_CONFIG: ", err)
 		return store

cli/config/configfile/file_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"encoding/json"
 	"errors"
+	"maps"
 	"os"
 	"testing"
 
@@ -584,6 +585,56 @@ func TestGetAllCredentialsFromEnvironment(t *testing.T) {
 	})
 }
 
+func TestGitlabCIWithDockerAuthConfig(t *testing.T) {
+	t.Setenv("GITLAB_CI", "true")
+
+	testAuthConfig := map[string]types.AuthConfig{
+		"env.example.test": {
+			Username: "user",
+			Password: "pass",
+		},
+	}
+	configFile := New("filename")
+	configFile.AuthConfigs = maps.Clone(testAuthConfig)
+
+	checkConfigFirst := func(t *testing.T) {
+		t.Helper()
+		authConfigs, err := configFile.GetAllCredentials()
+		assert.NilError(t, err)
+		assert.Check(t, is.DeepEqual(authConfigs, testAuthConfig))
+	}
+
+	checkEnvFirst := func(t *testing.T) {
+		t.Helper()
+		authConfigs, err := configFile.GetAllCredentials()
+		assert.NilError(t, err)
+		assert.Check(t, is.DeepEqual(authConfigs, map[string]types.AuthConfig{
+			"env.example.test": {
+				Username:      "env_user",
+				Password:      "env_pass",
+				ServerAddress: "env.example.test",
+			},
+		}))
+	}
+
+	t.Setenv("DOCKER_AUTH_CONFIG", envTestAuthConfig)
+	t.Run("config is used by default", func(t *testing.T) {
+		checkConfigFirst(t)
+	})
+	t.Run("config is used when configured explicitly", func(t *testing.T) {
+		configFile.Features = map[string]string{
+			"auth_config_order": "config,env",
+		}
+		checkConfigFirst(t)
+	})
+	t.Run("env is used if opt-in", func(t *testing.T) {
+		configFile.Features = map[string]string{
+			"auth_config_order": "env,config",
+		}
+		checkEnvFirst(t)
+	})
+}
+
 func TestParseEnvConfig(t *testing.T) {
 	t.Run("should error on unexpected fields", func(t *testing.T) {
 		_, err := parseEnvConfig(envTestUserPassConfig)

cli/config/memorystore/store.go

@@ -23,6 +23,7 @@ type Config struct {
 	lock              sync.RWMutex
 	memoryCredentials map[string]types.AuthConfig
 	fallbackStore     credentials.Store
+	preferFallback    bool
 }
 
 func (e *Config) Erase(serverAddress string) error {
@@ -43,6 +44,14 @@ func (e *Config) Erase(serverAddress string) error {
 func (e *Config) Get(serverAddress string) (types.AuthConfig, error) {
 	e.lock.RLock()
 	defer e.lock.RUnlock()
+
+	if e.preferFallback && e.fallbackStore != nil {
+		authConfig, err := e.fallbackStore.Get(serverAddress)
+		if err == nil {
+			return authConfig, nil
+		}
+	}
+
 	authConfig, ok := e.memoryCredentials[serverAddress]
 	if !ok {
 		if e.fallbackStore != nil {
@@ -58,16 +67,22 @@ func (e *Config) GetAll() (map[string]types.AuthConfig, error) {
 	defer e.lock.RUnlock()
 	creds := make(map[string]types.AuthConfig)
 
+	if e.preferFallback {
+		maps.Copy(creds, e.memoryCredentials)
+	}
+
 	if e.fallbackStore != nil {
 		fileCredentials, err := e.fallbackStore.GetAll()
 		if err != nil {
 			_, _ = fmt.Fprintln(os.Stderr, "memorystore: ", err)
 		} else {
-			creds = fileCredentials
+			maps.Copy(creds, fileCredentials)
 		}
 	}
 
-	maps.Copy(creds, e.memoryCredentials)
+	if !e.preferFallback {
+		maps.Copy(creds, e.memoryCredentials)
+	}
 	return creds, nil
 }
 
@@ -102,6 +117,26 @@ func WithFallbackStore(store credentials.Store) Options {
 	}
 }
 
+// WithPreferFallback configures the store to prefer reading credentials from
+// the fallback store.
+//
+// Write operations will be executed on both the memory store and the
+// fallback store.
+//
+// For read operations, the fallback store is checked first. If the credential
+// is not found there, the memory store is checked next.
+//
+// When retrieving all credentials, results from both the memory store and the
+// fallback store are combined into a single map.
+//
+// Credentials in the fallback store will override those in the memory store.
+func WithPreferFallback() Options {
+	return func(s *Config) error {
+		s.preferFallback = true
+		return nil
+	}
+}
+
 // WithAuthConfig allows to set the initial credentials in the memory store.
 func WithAuthConfig(config map[string]types.AuthConfig) Options {
 	return func(s *Config) error {

cli/config/memorystore/store_test.go

@@ -73,9 +73,9 @@ func TestMemoryStore(t *testing.T) {
 		err = memoryStore.Erase("https://a-new-credential.example.test")
 		assert.NilError(t, err)
 		_, err = memoryStore.Get("https://a-new-credential.example.test")
-		assert.Check(t, is.ErrorIs(err, errValueNotFound))
+		assert.Check(t, IsErrValueNotFound(err))
 		_, err = fallbackStore.Get("https://a-new-credential.example.test")
-		assert.Check(t, is.ErrorIs(err, errValueNotFound))
+		assert.Check(t, IsErrValueNotFound(err))
 	})
 }
 
@@ -126,6 +126,99 @@ func TestMemoryStoreWithoutFallback(t *testing.T) {
 		err = memoryStore.Erase("https://a-new-credential.example.test")
 		assert.NilError(t, err)
 		_, err = memoryStore.Get("https://a-new-credential.example.test")
+		assert.Check(t, IsErrValueNotFound(err))
+	})
+}
+
+func TestMemoryStoreWithPreferFallback(t *testing.T) {
+	config := map[string]types.AuthConfig{
+		"https://example.test": {
+			Username:      "memory-user",
+			ServerAddress: "https://example.test",
+			Auth:          "memory-token",
+		},
+		"https://only-in-memory.example.test": {
+			Username:      "only-memory-user",
+			ServerAddress: "https://only-in-memory.example.test",
+			Auth:          "only-memory-token",
+		},
+	}
+
+	fallbackConfig := map[string]types.AuthConfig{
+		"https://example.test": {
+			Username:      "fallback-user",
+			ServerAddress: "https://example.test",
+			Auth:          "fallback-token",
+		},
+		"https://only-in-file.example.test": {
+			Username:      "something-something",
+			ServerAddress: "https://only-in-file.example.test",
+			Auth:          "super_secret_token",
+		},
+	}
+
+	fallbackStore, err := New(WithAuthConfig(fallbackConfig))
+	assert.NilError(t, err)
+
+	memoryStore, err := New(WithAuthConfig(config), WithFallbackStore(fallbackStore), WithPreferFallback())
+	assert.NilError(t, err)
+
+	t.Run("get credentials prefers fallback store", func(t *testing.T) {
+		// should get from fallback
+		c, err := memoryStore.Get("https://example.test")
+		assert.NilError(t, err)
+		assert.Equal(t, c.Username, "fallback-user")
+
+		// should get from fallback (only exists there)
+		c, err = memoryStore.Get("https://only-in-file.example.test")
+		assert.NilError(t, err)
+		assert.Equal(t, c.Username, "something-something")
+
+		// should get from memory if not in fallback
+		c, err = memoryStore.Get("https://only-in-memory.example.test")
+		assert.NilError(t, err)
+		assert.Equal(t, c.Username, "only-memory-user")
+	})
+
+	t.Run("GetAll prefers fallback store", func(t *testing.T) {
+		all, err := memoryStore.GetAll()
+		assert.NilError(t, err)
+		assert.Equal(t, len(all), 3)
+		// value from fallback store should be present
+		assert.Equal(t, all["https://example.test"].Username, "fallback-user")
+		// value only in fallback should be present
+		assert.Equal(t, all["https://only-in-file.example.test"].Username, "something-something")
+		// value only in memory should be present
+		assert.Equal(t, all["https://only-in-memory.example.test"].Username, "only-memory-user")
+	})
+
+	t.Run("storing credentials writes to both stores", func(t *testing.T) {
+		newCred := types.AuthConfig{
+			Username:      "new-user",
+			ServerAddress: "https://new.example.test",
+			Auth:          "new-token",
+		}
+		err := memoryStore.Store(newCred)
+		assert.NilError(t, err)
+
+		// Check both stores to ensure the credential was written
+		c, err := memoryStore.Get("https://new.example.test")
+		assert.NilError(t, err)
+		assert.Equal(t, c, newCred)
+
+		c, err = fallbackStore.Get("https://new.example.test")
+		assert.NilError(t, err)
+		assert.Equal(t, c, newCred)
+	})
+
+	t.Run("Erase removes from both stores", func(t *testing.T) {
+		err := memoryStore.Erase("https://example.test")
+		assert.NilError(t, err)
+
+		_, err = memoryStore.Get("https://example.test")
+		assert.Check(t, is.ErrorIs(err, errValueNotFound))
+
+		_, err = fallbackStore.Get("https://example.test")
 		assert.Check(t, is.ErrorIs(err, errValueNotFound))
 	})
 }