Merge pull request #911 from sudo-bmitch/pr-validate-registry

Fix: Validate registry names

cmd/regctl/registry.go

@@ -225,6 +225,9 @@ func (registryOpts *registryCmd) runRegistryLogin(cmd *cobra.Command, args []str
 	if len(args) < 1 {
 		args = []string{regclient.DockerRegistry}
 	}
+	if !config.HostValidate(args[0]) {
+		return fmt.Errorf("invalid registry name provided: %s", args[0])
+	}
 	h := config.HostNewName(args[0])
 	if curH, ok := c.Hosts[h.Name]; ok {
 		h = curH
@@ -329,6 +332,9 @@ func (registryOpts *registryCmd) runRegistryLogout(cmd *cobra.Command, args []st
 	if len(args) < 1 {
 		args = []string{regclient.DockerRegistry}
 	}
+	if !config.HostValidate(args[0]) {
+		return fmt.Errorf("invalid registry name provided: %s", args[0])
+	}
 	h := config.HostNewName(args[0])
 	if curH, ok := c.Hosts[h.Name]; ok {
 		h = curH
@@ -360,6 +366,9 @@ func (registryOpts *registryCmd) runRegistrySet(cmd *cobra.Command, args []strin
 	if len(args) < 1 {
 		args = []string{regclient.DockerRegistry}
 	}
+	if !config.HostValidate(args[0]) {
+		return fmt.Errorf("invalid registry name provided: %s", args[0])
+	}
 	h := config.HostNewName(args[0])
 	if curH, ok := c.Hosts[h.Name]; ok {
 		h = curH

config/credhelper.go

@@ -86,6 +86,9 @@ func (ch *credHelper) list() ([]Host, error) {
 	}
 	hostList := []Host{}
 	for host, user := range credList {
+		if !HostValidate(host) {
+			continue
+		}
 		h := HostNewName(host)
 		h.User = user
 		h.CredHelper = ch.prog

config/credhelper_test.go

@@ -49,6 +49,7 @@ func TestCredHelper(t *testing.T) {
 		},
 		{
 			name:       "missing helper",
+			host:       "missing.example.org",
 			credHelper: "./testdata/docker-credential-missing",
 			expectErr:  true,
 		},

config/docker.go

@@ -85,6 +85,9 @@ func dockerParse(cf *conffile.File) ([]Host, error) {
 	}
 	hosts := []Host{}
 	for name, auth := range dc.AuthConfigs {
+		if !HostValidate(name) {
+			continue
+		}
 		h, err := dockerAuthToHost(name, dc, auth)
 		if err != nil {
 			continue
@@ -93,9 +96,12 @@ func dockerParse(cf *conffile.File) ([]Host, error) {
 	}
 	// also include default entries for credential helpers
 	for name, helper := range dc.CredentialHelpers {
+		if !HostValidate(name) {
+			continue
+		}
 		h := HostNewName(name)
 		h.CredHelper = dockerHelperPre + helper
-		if _, ok := dc.AuthConfigs[h.Name]; ok {
+		if _, ok := dc.AuthConfigs[name]; ok {
 			continue // skip fields with auth config
 		}
 		hosts = append(hosts, *h)

config/docker_test.go

@@ -86,22 +86,42 @@ func TestDocker(t *testing.T) {
 		},
 		{
 			name:          "missing-from-repo.example.com", // entries with a repository are ignored
+			hostname:      "missing-from-repo.example.com",
 			expectMissing: true,
 		},
 		{
 			name:          "index.docker.io", // verify access-token and refresh-token entries are ignored
+			hostname:      "index.docker.io",
+			expectMissing: true,
+		},
+		{
+			name:          "https://index.docker.io/v1/access-token",
+			hostname:      "https://index.docker.io/v1/access-token",
+			expectMissing: true,
+		},
+		{
+			name:          "https://index.docker.io/v1/test-token",
+			hostname:      "https://index.docker.io/v1/test-token",
+			expectMissing: true,
+		},
+		{
+			name:          "https://index.docker.io/v1/helper-token",
+			hostname:      "https://index.docker.io/v1/helper-token",
 			expectMissing: true,
 		},
 	}
 	for _, tc := range tt {
 		t.Run(tc.name, func(t *testing.T) {
 			h, ok := hostMap[tc.hostname]
-			if !ok {
-				if !tc.expectMissing {
-					t.Fatalf("host not found: %s", tc.hostname)
+			if tc.expectMissing {
+				if ok {
+					t.Fatalf("entry found that should be missing: %s", tc.hostname)
 				}
 				return
 			}
+			if !ok {
+				t.Fatalf("host not found: %s", tc.hostname)
+			}
 			if tc.expectUser != h.User {
 				t.Errorf("user mismatch, expect %s, received %s", tc.expectUser, h.User)
 			}

config/host.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io"
 	"log/slog"
+	"maps"
 	"slices"
 	"strings"
 	"time"
@@ -141,6 +142,11 @@ func HostNew() *Host {
 	return &h
 }
 
+// HostNewName creates a default Host with a hostname.
+func HostNewName(name string) *Host {
+	return HostNewDefName(nil, name)
+}
+
 // HostNewDefName creates a host using provided defaults and hostname.
 func HostNewDefName(def *Host, name string) *Host {
 	var h Host
@@ -176,39 +182,29 @@ func HostNewDefName(def *Host, name string) *Host {
 		}
 	}
 	// configure host
-	origName := name
+	scheme, registry, _ := parseName(name)
+	if scheme == "http" {
+		h.TLS = TLSDisabled
+	}
 	// Docker Hub is a special case
-	if name == DockerRegistryAuth || name == DockerRegistryDNS || name == DockerRegistry {
+	if registry == DockerRegistry {
 		h.Name = DockerRegistry
 		h.Hostname = DockerRegistryDNS
 		h.CredHost = DockerRegistryAuth
 		return &h
 	}
-	// handle http/https prefix
-	i := strings.Index(name, "://")
-	if i > 0 {
-		scheme := name[:i]
-		name = name[i+3:]
-		if scheme == "http" {
-			h.TLS = TLSDisabled
-		}
-	}
-	// trim any repository path
-	i = strings.Index(name, "/")
-	if i > 0 {
-		name = name[:i]
-	}
-	h.Name = name
-	h.Hostname = name
-	if origName != name {
-		h.CredHost = origName
+	h.Name = registry
+	h.Hostname = registry
+	if name != registry {
+		h.CredHost = name
 	}
 	return &h
 }
 
-// HostNewName creates a default Host with a hostname.
-func HostNewName(name string) *Host {
-	return HostNewDefName(nil, name)
+// HostValidate returns true if the scheme is missing or a known value, and the path is not set.
+func HostValidate(name string) bool {
+	scheme, _, path := parseName(name)
+	return path == "" && (scheme == "https" || scheme == "http")
 }
 
 // GetCred returns the credential, fetching from a credential helper if needed.
@@ -444,7 +440,7 @@ func (host *Host) Merge(newHost Host, log *slog.Logger) error {
 
 	if len(newHost.APIOpts) > 0 {
 		if len(host.APIOpts) > 0 {
-			merged := copyMapString(host.APIOpts)
+			merged := maps.Clone(host.APIOpts)
 			for k, v := range newHost.APIOpts {
 				if host.APIOpts[k] != "" && host.APIOpts[k] != v {
 					log.Warn("Changing APIOpts setting for registry",
@@ -504,10 +500,25 @@ func (host *Host) Merge(newHost Host, log *slog.Logger) error {
 	return nil
 }
 
-func copyMapString(src map[string]string) map[string]string {
-	copy := map[string]string{}
-	for k, v := range src {
-		copy[k] = v
+// parseName splits a registry into the scheme, hostname, and repository/path.
+func parseName(name string) (string, string, string) {
+	scheme := "https"
+	path := ""
+	// Docker Hub is a special case
+	if name == DockerRegistryAuth || name == DockerRegistryDNS || name == DockerRegistry {
+		return scheme, DockerRegistry, ""
 	}
-	return copy
+	// handle http/https prefix
+	i := strings.Index(name, "://")
+	if i > 0 {
+		scheme = name[:i]
+		name = name[i+3:]
+	}
+	// trim any repository path
+	i = strings.Index(name, "/")
+	if i > 0 {
+		path = name[i+1:]
+		name = name[:i]
+	}
+	return scheme, name, path
 }

config/testdata/config.json

@@ -19,7 +19,9 @@
   "credHelpers": {
     "testhost.example.com": "test",
     "https://index.docker.io/v1/": "test",
-    "http://http.example.com/": "test"
+    "http://http.example.com/": "test",
+    "https://index.docker.io/v1/test-token": "test",
+    "https://index.docker.io/v1/access-token": "test"
   },
   "credsStore": "teststore"
 }

config/testdata/docker-credential-teststore

@@ -3,7 +3,8 @@
 list='{
   "http://storehttp.example.com/": "hello",
   "storehost.example.com": "hello",
-  "storetoken.example.com": "<token>"
+  "storetoken.example.com": "<token>",
+  "https://index.docker.io/v1/helper-token": "<token>"
 }'
 
 registry_http='
@@ -24,6 +25,12 @@ registry_testtoken='
   "Secret": "deadbeefcafe"
 }
 '
+registry_testhelper_token='
+{ "ServerURL": "https://index.docker.io/v1/helper-token",
+  "Username": "<token>",
+  "Secret": "deadbeefcafe"
+}
+'
 
 if [ "$1" = "get" ]; then
   read hostname
@@ -40,6 +47,10 @@ if [ "$1" = "get" ]; then
       echo "${registry_testtoken}"
       exit 0
       ;;
+    https://index.docker.io/v1/helper-token)
+      echo "${registry_testhelper_token}"
+      exit 0
+      ;;
   esac
 elif [ "$1" = "list" ]; then
   echo "${list}"