[gitpod-protocol] handle host:port:token for getGitpodImageAuth (#20806)

kylos101 · Copilot · geropl · web-flow · commit 6f3319ea59be · 2025-05-13T03:11:58.000-04:00
* [gitpod-protocol] handle host:token and host:port:token for getGitpodImageAuth

* Cleanup

* Improve readability

* Code review feedback

* Update components/gitpod-protocol/src/protocol.ts

Co-authored-by: Copilot &lt;175728472+Copilot@users.noreply.github.com&gt;

* [supervisor] tests to cover insertCredentialsIntoConfig

* Fix tests

* [supervisor] handle auth token like `host:port:token`

* [image-builder-mk3] handle host:port:token for auth

* Fix

* Cleanup

* Cleanup

* Cleanup

* Cleanup

* [image-builder-bob] tolerate host:port:token

Special case is to strip port for 443

* Handle center values &amp; code review feedback

* Remove extra/unnecessary trim

* [bob] proxy: explicit and implicit fallback for exact header matching for auth proxy

---------

Co-authored-by: Copilot &lt;175728472+Copilot@users.noreply.github.com&gt;
Co-authored-by: Gero Posmyk-Leinemann &lt;gero@gitpod.io&gt;
diff --git a/components/gitpod-protocol/src/protocol.spec.ts b/components/gitpod-protocol/src/protocol.spec.ts
@@ -6,7 +6,7 @@
 
 import { suite, test } from "@testdeck/mocha";
 import * as chai from "chai";
-import { SSHPublicKeyValue } from ".";
+import { SSHPublicKeyValue, EnvVar, EnvVarWithValue } from ".";
 
 const expect = chai.expect;
 
@@ -94,4 +94,120 @@ class TestSSHPublicKeyValue {
         ).to.throw("Key is invalid");
     }
 }
-module.exports = new TestSSHPublicKeyValue(); // Only to circumvent no usage warning :-/
+
+@suite
+class TestEnvVar {
+    @test
+    public testGetGitpodImageAuth_empty() {
+        const result = EnvVar.getGitpodImageAuth([]);
+        expect(result.size).to.equal(0);
+    }
+
+    @test
+    public testGetGitpodImageAuth_noRelevantVar() {
+        const envVars: EnvVarWithValue[] = [{ name: "OTHER_VAR", value: "some_value" }];
+        const result = EnvVar.getGitpodImageAuth(envVars);
+        expect(result.size).to.equal(0);
+    }
+
+    @test
+    public testGetGitpodImageAuth_singleEntryNoPort() {
+        const envVars: EnvVarWithValue[] = [
+            {
+                name: EnvVar.GITPOD_IMAGE_AUTH_ENV_VAR_NAME,
+                value: "my-registry.foo.net:Zm9vOmJhcg==",
+            },
+        ];
+        const result = EnvVar.getGitpodImageAuth(envVars);
+        expect(result.size).to.equal(1);
+        expect(result.get("my-registry.foo.net")).to.equal("Zm9vOmJhcg==");
+    }
+
+    @test
+    public testGetGitpodImageAuth_singleEntryWithPort() {
+        const envVars: EnvVarWithValue[] = [
+            {
+                name: EnvVar.GITPOD_IMAGE_AUTH_ENV_VAR_NAME,
+                value: "my-registry.foo.net:5000:Zm9vOmJhcg==",
+            },
+        ];
+        const result = EnvVar.getGitpodImageAuth(envVars);
+        expect(result.size).to.equal(1);
+        expect(result.get("my-registry.foo.net:5000")).to.equal("Zm9vOmJhcg==");
+    }
+
+    @test
+    public testGetGitpodImageAuth_multipleEntries() {
+        const envVars: EnvVarWithValue[] = [
+            {
+                name: EnvVar.GITPOD_IMAGE_AUTH_ENV_VAR_NAME,
+                value: "my-registry.foo.net:Zm9vOmJhcg==,my-registry2.bar.com:YWJjOmRlZg==",
+            },
+        ];
+        const result = EnvVar.getGitpodImageAuth(envVars);
+        expect(result.size).to.equal(2);
+        expect(result.get("my-registry.foo.net")).to.equal("Zm9vOmJhcg==");
+        expect(result.get("my-registry2.bar.com")).to.equal("YWJjOmRlZg==");
+    }
+
+    @test
+    public testGetGitpodImageAuth_multipleEntriesWithPortAndMalformed() {
+        const envVars: EnvVarWithValue[] = [
+            {
+                name: EnvVar.GITPOD_IMAGE_AUTH_ENV_VAR_NAME,
+                value: "my-registry.foo.net:5000:Zm9vOmJhcg==,my-registry2.bar.com:YWJjOmRlZg==,invalidEntry,another.host:anothercred",
+            },
+        ];
+        const result = EnvVar.getGitpodImageAuth(envVars);
+        expect(result.size).to.equal(3);
+        expect(result.get("my-registry2.bar.com")).to.equal("YWJjOmRlZg==");
+        expect(result.get("another.host")).to.equal("anothercred");
+        expect(result.get("my-registry.foo.net:5000")).to.equal("Zm9vOmJhcg==");
+    }
+
+    @test
+    public testGetGitpodImageAuth_emptyValue() {
+        const envVars: EnvVarWithValue[] = [
+            {
+                name: EnvVar.GITPOD_IMAGE_AUTH_ENV_VAR_NAME,
+                value: "",
+            },
+        ];
+        const result = EnvVar.getGitpodImageAuth(envVars);
+        expect(result.size).to.equal(0);
+    }
+
+    @test
+    public testGetGitpodImageAuth_malformedEntries() {
+        const envVars: EnvVarWithValue[] = [
+            {
+                name: EnvVar.GITPOD_IMAGE_AUTH_ENV_VAR_NAME,
+                value: "justhost,hostonly:,:credonly,:::,:,",
+            },
+        ];
+        const result = EnvVar.getGitpodImageAuth(envVars);
+        expect(result.size).to.equal(0);
+    }
+
+    @test
+    public testGetGitpodImageAuth_entriesWithSpaces() {
+        const envVars: EnvVarWithValue[] = [
+            {
+                name: EnvVar.GITPOD_IMAGE_AUTH_ENV_VAR_NAME,
+                value: " my-registry.foo.net : Zm9vOmJhcg== ,  my-registry2.bar.com:YWJjOmRlZg==  ",
+            },
+        ];
+        const result = EnvVar.getGitpodImageAuth(envVars);
+        expect(result.size).to.equal(2);
+        expect(result.get("my-registry.foo.net")).to.equal("Zm9vOmJhcg==");
+        expect(result.get("my-registry2.bar.com")).to.equal("YWJjOmRlZg==");
+    }
+}
+
+// Exporting both test suites
+const testSSHPublicKeyValue = new TestSSHPublicKeyValue();
+const testEnvVar = new TestEnvVar();
+module.exports = {
+    testSSHPublicKeyValue,
+    testEnvVar,
+};
diff --git a/components/gitpod-protocol/src/protocol.ts b/components/gitpod-protocol/src/protocol.ts
@@ -293,11 +293,28 @@ export namespace EnvVar {
             return res;
         }
 
+        const parse = (parts: string[]): { host: string; auth: string } | undefined => {
+            if (parts.some((e) => e === "")) {
+                return undefined;
+            }
+            if (parts.length === 2) {
+                return { host: parts[0], auth: parts[1] };
+            } else if (parts.length === 3) {
+                return { host: `${parts[0]}:${parts[1]}`, auth: parts[2] };
+            }
+            return undefined;
+        };
+
         (imageAuth.value || "")
             .split(",")
-            .map((e) => e.trim().split(":"))
-            .filter((e) => e.length == 2)
-            .forEach((e) => res.set(e[0], e[1]));
+            .map((e) => e.split(":").map((part) => part.trim()))
+            .forEach((parts) => {
+                const parsed = parse(parts);
+                if (parsed) {
+                    res.set(parsed.host, parsed.auth);
+                }
+            });
+
         return res;
     }
 }
diff --git a/components/image-builder-bob/pkg/proxy/auth.go b/components/image-builder-bob/pkg/proxy/auth.go
@@ -44,30 +44,50 @@ type authConfig struct {
 
 type MapAuthorizer map[string]authConfig
 
-func (a MapAuthorizer) Authorize(host string) (user, pass string, err error) {
+func (a MapAuthorizer) Authorize(hostHeader string) (user, pass string, err error) {
 	defer func() {
 		log.WithFields(logrus.Fields{
-			"host": host,
+			"host": hostHeader,
 			"user": user,
 		}).Info("authorizing registry access")
 	}()
 
-	// Strip any port from the host if present
-	host = strings.Split(host, ":")[0]
+	parseHostHeader := func(hostHeader string) (string, string) {
+		hostHeaderSlice := strings.Split(hostHeader, ":")
+		hostname := strings.TrimSpace(hostHeaderSlice[0])
+		var port string
+		if len(hostHeaderSlice) > 1 {
+			port = strings.TrimSpace(hostHeaderSlice[1])
+		}
+		return hostname, port
+	}
+	hostname, port := parseHostHeader(hostHeader)
+	// gpl: Could be port 80 as well, but we don't know if we are servinc http or https, we assume https
+	if port == "" {
+		port = "443"
+	}
+	host := hostname + ":" + port
 
 	explicitHostMatcher := func() (authConfig, bool) {
+		// 1. precise host match
 		res, ok := a[host]
+		if ok {
+			return res, ok
+		}
+
+		// 2. make sure we not have a hostname match
+		res, ok = a[hostname]
 		return res, ok
 	}
 	ecrHostMatcher := func() (authConfig, bool) {
-		if isECRRegistry(host) {
+		if isECRRegistry(hostname) {
 			res, ok := a[DummyECRRegistryDomain]
 			return res, ok
 		}
 		return authConfig{}, false
 	}
 	dockerHubHostMatcher := func() (authConfig, bool) {
-		if isDockerHubRegistry(host) {
+		if isDockerHubRegistry(hostname) {
 			res, ok := a["docker.io"]
 			return res, ok
 		}
diff --git a/components/image-builder-bob/pkg/proxy/auth_test.go b/components/image-builder-bob/pkg/proxy/auth_test.go
@@ -32,7 +32,7 @@ func TestAuthorize(t *testing.T) {
 			},
 		},
 		{
-			name:        "docker auth format - valid credentials - host with port",
+			name:        "docker auth format - valid credentials - host with :443 port",
 			constructor: NewAuthorizerFromDockerEnvVar,
 			input:       `{"auths": {"registry.example.com": {"auth": "dXNlcjpwYXNz"}}}`, // base64(user:pass)
 			testHost:    "registry.example.com:443",
@@ -110,6 +110,26 @@ func TestAuthorize(t *testing.T) {
 				pass: "",
 			},
 		},
+		{
+			name:        "Docker Hub",
+			constructor: NewAuthorizerFromEnvVar,
+			input:       `{"docker.io": {"auth": "dXNlcjpwYXNz"}}`,
+			testHost:    "registry-1.docker.io",
+			expected: expectation{
+				user: "user",
+				pass: "pass",
+			},
+		},
+		{
+			name:        "docker auth format - valid credentials - host with :5000 port",
+			constructor: NewAuthorizerFromDockerEnvVar,
+			input:       `{"auths": {"registry.example.com:5000": {"auth": "dXNlcjpwYXNz"}}}`, // base64(user:pass)
+			testHost:    "registry.example.com:5000",
+			expected: expectation{
+				user: "user",
+				pass: "pass",
+			},
+		},
 	}
 
 	for _, tt := range tests {
diff --git a/components/image-builder-mk3/pkg/auth/auth.go b/components/image-builder-mk3/pkg/auth/auth.go
@@ -383,9 +383,11 @@ func (a AllowedAuthFor) additionalAuth(domain string) *Authentication {
 	dec, err := base64.StdEncoding.DecodeString(ath)
 	if err == nil {
 		segs := strings.Split(string(dec), ":")
-		if len(segs) > 1 {
-			res.Username = segs[0]
-			res.Password = strings.Join(segs[1:], ":")
+		numSegs := len(segs)
+
+		if numSegs > 1 {
+			res.Username = strings.Join(segs[:numSegs-1], ":")
+			res.Password = segs[numSegs-1]
 		}
 	} else {
 		log.Errorf("failed getting additional auth")
diff --git a/components/image-builder-mk3/pkg/auth/auth_test.go b/components/image-builder-mk3/pkg/auth/auth_test.go
@@ -5,6 +5,7 @@
 package auth
 
 import (
+	"encoding/base64"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -30,3 +31,98 @@ func TestIsECRRegistry(t *testing.T) {
 		})
 	}
 }
+
+func TestAdditionalAuth(t *testing.T) {
+	tests := []struct {
+		name          string
+		domain        string
+		additionalMap map[string]string
+		expectedAuth  *Authentication
+	}{
+		{
+			name:   "standard host:token",
+			domain: "myregistry.com",
+			additionalMap: map[string]string{
+				"myregistry.com": base64.StdEncoding.EncodeToString([]byte("myregistry.com:mytoken")),
+			},
+			expectedAuth: &Authentication{
+				Username: "myregistry.com",
+				Password: "mytoken",
+				Auth:     base64.StdEncoding.EncodeToString([]byte("myregistry.com:mytoken")),
+			},
+		},
+		{
+			name:   "buggy host:port:token",
+			domain: "myregistry.com:5000",
+			additionalMap: map[string]string{
+				"myregistry.com:5000": base64.StdEncoding.EncodeToString([]byte("myregistry.com:5000:mytoken")),
+			},
+			expectedAuth: &Authentication{
+				Username: "myregistry.com:5000",
+				Password: "mytoken",
+				Auth:     base64.StdEncoding.EncodeToString([]byte("myregistry.com:5000:mytoken")),
+			},
+		},
+		{
+			name:   "only username, no password/token (single segment)",
+			domain: "useronly.com",
+			additionalMap: map[string]string{
+				"useronly.com": base64.StdEncoding.EncodeToString([]byte("justauser")),
+			},
+			expectedAuth: &Authentication{
+				Auth: base64.StdEncoding.EncodeToString([]byte("justauser")),
+			},
+		},
+		{
+			name:   "empty auth string",
+			domain: "emptyauth.com",
+			additionalMap: map[string]string{
+				"emptyauth.com": base64.StdEncoding.EncodeToString([]byte("")),
+			},
+			expectedAuth: &Authentication{
+				Auth: base64.StdEncoding.EncodeToString([]byte("")),
+			},
+		},
+		{
+			name:          "domain not in map",
+			domain:        "notfound.com",
+			additionalMap: map[string]string{"someother.com": base64.StdEncoding.EncodeToString([]byte("someauth"))},
+			expectedAuth:  nil,
+		},
+		{
+			name:   "invalid base64 string",
+			domain: "invalidbase64.com",
+			additionalMap: map[string]string{
+				"invalidbase64.com": "!!!INVALID_BASE64!!!",
+			},
+			expectedAuth: &Authentication{
+				Auth: "!!!INVALID_BASE64!!!",
+			},
+		},
+		{
+			name:   "standard host:token where username in cred is different from domain key",
+			domain: "docker.io",
+			additionalMap: map[string]string{
+				"docker.io": base64.StdEncoding.EncodeToString([]byte("user1:pass1")),
+			},
+			expectedAuth: &Authentication{
+				Username: "user1",
+				Password: "pass1",
+				Auth:     base64.StdEncoding.EncodeToString([]byte("user1:pass1")),
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			aaf := AllowedAuthFor{
+				Additional: tt.additionalMap,
+			}
+			actualAuth := aaf.additionalAuth(tt.domain)
+
+			if diff := cmp.Diff(tt.expectedAuth, actualAuth); diff != "" {
+				t.Errorf("additionalAuth() mismatch (-want +got):\n%s", diff)
+			}
+		})
+	}
+}
diff --git a/components/supervisor/pkg/supervisor/docker.go b/components/supervisor/pkg/supervisor/docker.go
diff --git a/components/supervisor/pkg/supervisor/docker_test.go b/components/supervisor/pkg/supervisor/docker_test.go
