From 2e3d144aca2f7aa96cbe3a18c43775cc598b6eb1 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Sat, 19 Sep 2026 18:56:46 -0400 Subject: [PATCH] fix: erase the docker hub credential on registry:logout Docker stores Docker Hub under the index server address rather than the hostname the user logged in with, and `docker logout` only erases the exact key it is given, so `registry:logout docker.io` reported success while leaving the credential in place. The server is now resolved to the key docker actually stores before the logout runs. --- plugins/registry/functions.go | 48 ++++++++++++++++++ plugins/registry/functions_test.go | 79 ++++++++++++++++++++++++++++++ plugins/registry/subcommands.go | 11 ++--- tests.mk | 1 + tests/unit/registry.bats | 12 +++++ 5 files changed, 145 insertions(+), 6 deletions(-) create mode 100644 plugins/registry/functions_test.go diff --git a/plugins/registry/functions.go b/plugins/registry/functions.go index bad7f668b..d98dd2a76 100644 --- a/plugins/registry/functions.go +++ b/plugins/registry/functions.go @@ -15,6 +15,9 @@ import ( const registryConfigDir = "/var/lib/dokku/config/registry" +// dockerIndexServer is the auths key docker stores Docker Hub credentials under +const dockerIndexServer = "https://index.docker.io/v1/" + // GetAppRegistryConfigDir returns the per-app registry config directory func GetAppRegistryConfigDir(appName string) string { return filepath.Join(registryConfigDir, appName) @@ -30,9 +33,19 @@ func GetComputedAppRegistryConfigDir(appName string) string { return GetAppRegistryConfigDir(appName) } + return GetGlobalRegistryConfigDir() +} + +// GetGlobalRegistryConfigDir returns the docker config directory global logins write to +func GetGlobalRegistryConfigDir() string { return filepath.Join(os.Getenv("DOKKU_ROOT"), ".docker") } +// GetGlobalRegistryConfigPath returns the path to the global docker config.json +func GetGlobalRegistryConfigPath() string { + return filepath.Join(GetGlobalRegistryConfigDir(), "config.json") +} + // HasAppRegistryAuth checks if an app has registry credentials configured func HasAppRegistryAuth(appName string) bool { configPath := GetAppRegistryConfigPath(appName) @@ -245,3 +258,38 @@ func imageCleanup(appName string, imageRepo string, imageTag string, tag int) { imagesToRemove = append(imagesToRemove, imageIDs...) common.RemoveImages(imagesToRemove) } + +// normalizeRegistryServer applies the dokku-level aliases for a registry server +func normalizeRegistryServer(server string) string { + if server == "hub.docker.com" || server == "docker.com" { + return "docker.io" + } + + return server +} + +// convertToHostname strips any scheme and path from a registry server, matching +// the rule docker applies before storing or looking up a credential +func convertToHostname(server string) string { + stripped := server + if strings.HasPrefix(stripped, "http://") { + stripped = strings.TrimPrefix(stripped, "http://") + } else if strings.HasPrefix(stripped, "https://") { + stripped = strings.TrimPrefix(stripped, "https://") + } + + return strings.SplitN(stripped, "/", 2)[0] +} + +// dockerAuthKey returns the key docker uses for a registry server in the auths +// map of a config.json. Docker Hub is stored under the index server address +// rather than the hostname the user logged in with, and is also the only server +// docker treats that way: registry-1.docker.io gets a key of its own. +func dockerAuthKey(server string) string { + hostname := convertToHostname(normalizeRegistryServer(server)) + if hostname == "" || hostname == "docker.io" || hostname == "index.docker.io" { + return dockerIndexServer + } + + return hostname +} diff --git a/plugins/registry/functions_test.go b/plugins/registry/functions_test.go new file mode 100644 index 000000000..0a8812f90 --- /dev/null +++ b/plugins/registry/functions_test.go @@ -0,0 +1,79 @@ +package registry + +import "testing" + +func TestNormalizeRegistryServer(t *testing.T) { + cases := []struct { + name string + in string + want string + }{ + {"hub.docker.com alias", "hub.docker.com", "docker.io"}, + {"docker.com alias", "docker.com", "docker.io"}, + {"docker.io untouched", "docker.io", "docker.io"}, + {"other registry untouched", "ghcr.io", "ghcr.io"}, + {"empty untouched", "", ""}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := normalizeRegistryServer(tc.in); got != tc.want { + t.Errorf("normalizeRegistryServer(%q) = %q, want %q", tc.in, got, tc.want) + } + }) + } +} + +func TestConvertToHostname(t *testing.T) { + cases := []struct { + name string + in string + want string + }{ + {"bare hostname", "ghcr.io", "ghcr.io"}, + {"https scheme", "https://ghcr.io", "ghcr.io"}, + {"http scheme", "http://ghcr.io", "ghcr.io"}, + {"scheme and path", "https://index.docker.io/v1/", "index.docker.io"}, + {"trailing slash", "ghcr.io/", "ghcr.io"}, + {"port preserved", "localhost:5000", "localhost:5000"}, + {"port with scheme and path", "https://localhost:5000/v2/", "localhost:5000"}, + {"empty", "", ""}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := convertToHostname(tc.in); got != tc.want { + t.Errorf("convertToHostname(%q) = %q, want %q", tc.in, got, tc.want) + } + }) + } +} + +func TestDockerAuthKey(t *testing.T) { + cases := []struct { + name string + in string + want string + }{ + {"docker.io", "docker.io", dockerIndexServer}, + {"hub.docker.com", "hub.docker.com", dockerIndexServer}, + {"docker.com", "docker.com", dockerIndexServer}, + {"index.docker.io", "index.docker.io", dockerIndexServer}, + {"index server address", dockerIndexServer, dockerIndexServer}, + {"empty server", "", dockerIndexServer}, + // docker does not treat registry-1.docker.io as the official index and + // stores it under a key of its own + {"registry-1.docker.io is not the index", "registry-1.docker.io", "registry-1.docker.io"}, + {"other registry", "ghcr.io", "ghcr.io"}, + {"other registry with scheme", "https://ghcr.io/v2/", "ghcr.io"}, + {"registry with port", "localhost:5000", "localhost:5000"}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := dockerAuthKey(tc.in); got != tc.want { + t.Errorf("dockerAuthKey(%q) = %q, want %q", tc.in, got, tc.want) + } + }) + } +} diff --git a/plugins/registry/subcommands.go b/plugins/registry/subcommands.go index 6c97ebd2f..747dd463e 100644 --- a/plugins/registry/subcommands.go +++ b/plugins/registry/subcommands.go @@ -32,9 +32,7 @@ func CommandLogin(appName string, server string, username string, password strin return errors.New("Missing password argument") } - if server == "hub.docker.com" || server == "docker.com" { - server = "docker.io" - } + server = normalizeRegistryServer(server) buffer := bytes.Buffer{} buffer.Write([]byte(password + "\n")) @@ -92,9 +90,10 @@ func CommandLogout(appName string, server string) error { return errors.New("Missing server argument") } - if server == "hub.docker.com" || server == "docker.com" { - server = "docker.io" - } + // docker only erases the exact auths key it is given, and stores Docker Hub + // under the index server address rather than the hostname logged in with, so + // a logout for docker.io has to name that address to remove anything + server = dockerAuthKey(server) env := map[string]string{} if appName != "" { diff --git a/tests.mk b/tests.mk index 1548c8f52..f68f4688f 100644 --- a/tests.mk +++ b/tests.mk @@ -186,6 +186,7 @@ go-tests: @$(MAKE) go-test-plugin PLUGIN_NAME=logs @$(MAKE) go-test-plugin PLUGIN_NAME=network @$(MAKE) go-test-plugin PLUGIN_NAME=ps + @$(MAKE) go-test-plugin PLUGIN_NAME=registry @$(MAKE) go-test-plugin PLUGIN_NAME=buildpacks @$(MAKE) go-test-plugin PLUGIN_NAME=scheduler-k3s @$(MAKE) go-test-plugin PLUGIN_NAME=storage diff --git a/tests/unit/registry.bats b/tests/unit/registry.bats index 249676752..a21802a20 100644 --- a/tests/unit/registry.bats +++ b/tests/unit/registry.bats @@ -90,6 +90,12 @@ teardown() { echo "output: $output" echo "status: $status" assert_success + + run /bin/bash -c "jq -r '.auths | keys | join(\",\")' /var/lib/dokku/config/registry/$TEST_APP/config.json" + echo "output: $output" + echo "status: $status" + assert_success + assert_output_not_contains "index.docker.io" } @test "(registry:logout) global logout" { @@ -106,6 +112,12 @@ teardown() { echo "output: $output" echo "status: $status" assert_success + + run /bin/bash -c "jq -r '.auths | keys | join(\",\")' $DOKKU_ROOT/.docker/config.json" + echo "output: $output" + echo "status: $status" + assert_success + assert_output_not_contains "index.docker.io" } @test "(registry) per-app credentials deleted on app destroy" {