fix: repo credentials not resolved when two aliases share the same url

Signed-off-by: Denis Nutiu <dnutiu@hey.com>

pkg/downloader/chart_downloader.go

@@ -85,6 +85,13 @@ type ChartDownloader struct {
 
 	// Cache specifies the cache implementation to use.
 	Cache Cache
+
+	// CredentialsResolved indicates that credentials (username/password) have
+	// already been set in Options and should not be overridden by repository
+	// scanning in ResolveChartVersion. This prevents incorrect credential
+	// selection when multiple repositories share the same URL with different
+	// authentication details.
+	CredentialsResolved bool
 }
 
 // DownloadTo retrieves a chart. Depending on the settings, it may also download a provenance file.
@@ -410,7 +417,7 @@ func (c *ChartDownloader) ResolveChartVersion(ref, version string) (string, *url
 		if rc.CertFile != "" || rc.KeyFile != "" || rc.CAFile != "" {
 			c.Options = append(c.Options, getter.WithTLSClientConfig(rc.CertFile, rc.KeyFile, rc.CAFile))
 		}
-		if rc.Username != "" && rc.Password != "" {
+		if rc.Username != "" && rc.Password != "" && !c.CredentialsResolved {
 			c.Options = append(
 				c.Options,
 				getter.WithBasicAuth(rc.Username, rc.Password),

pkg/downloader/manager.go

@@ -113,7 +113,8 @@ func (m *Manager) Build() error {
 		}
 	}
 
-	if _, err := m.resolveRepoNames(req); err != nil {
+	repoNames, err := m.resolveRepoNames(req)
+	if err != nil {
 		return err
 	}
 
@@ -144,7 +145,7 @@ func (m *Manager) Build() error {
 	}
 
 	// Now we need to fetch every package here into charts/
-	return m.downloadAll(lock.Dependencies)
+	return m.downloadAll(lock.Dependencies, repoNames)
 }
 
 // Update updates a local charts directory.
@@ -200,7 +201,7 @@ func (m *Manager) Update() error {
 	}
 
 	// Now we need to fetch every package here into charts/
-	if err := m.downloadAll(lock.Dependencies); err != nil {
+	if err := m.downloadAll(lock.Dependencies, repoNames); err != nil {
 		return err
 	}
 
@@ -242,7 +243,7 @@ func (m *Manager) resolve(req []*chart.Dependency, repoNames map[string]string)
 //
 // It will delete versions of the chart that exist on disk and might cause
 // a conflict.
-func (m *Manager) downloadAll(deps []*chart.Dependency) error {
+func (m *Manager) downloadAll(deps []*chart.Dependency, repoNames map[string]string) error {
 	repos, err := m.loadChartRepositories()
 	if err != nil {
 		return err
@@ -315,7 +316,8 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error {
 
 		// Any failure to resolve/download a chart should fail:
 		// https://github.com/helm/helm/issues/1439
-		churl, username, password, insecureSkipTLSVerify, passCredentialsAll, caFile, certFile, keyFile, err := m.findChartURL(dep.Name, dep.Version, dep.Repository, repos)
+		repoName := repoNames[dep.Name]
+		churl, username, password, insecureSkipTLSVerify, passCredentialsAll, caFile, certFile, keyFile, err := m.findChartURL(dep.Name, dep.Version, dep.Repository, repoName, repos)
 		if err != nil {
 			saveError = fmt.Errorf("could not find %s: %w", churl, err)
 			break
@@ -343,6 +345,7 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error {
 				getter.WithInsecureSkipVerifyTLS(insecureSkipTLSVerify),
 				getter.WithTLSClientConfig(certFile, keyFile, caFile),
 			},
+			CredentialsResolved: true,
 		}
 
 		version := ""
@@ -719,14 +722,45 @@ func (m *Manager) parallelRepoUpdate(repos []*repo.Entry) error {
 // 'name' is the name of the chart. Version is an exact semver, or an empty string. If empty, the
 // newest version will be returned.
 //
-// repoURL is the repository to search
+// repoURL is the repository to search.
+//
+// repoName is the name (alias) of the repository as configured in repositories.yaml.
+// When provided, the repository is looked up by name rather than by URL, ensuring that
+// the correct credentials are used when multiple repositories share the same URL.
 //
 // If it finds a URL that is "relative", it will prepend the repoURL.
-func (m *Manager) findChartURL(name, version, repoURL string, repos map[string]*repo.ChartRepository) (url, username, password string, insecureSkipTLSVerify, passCredentialsAll bool, caFile, certFile, keyFile string, err error) {
+func (m *Manager) findChartURL(name, version, repoURL, repoName string, repos map[string]*repo.ChartRepository) (url, username, password string, insecureSkipTLSVerify, passCredentialsAll bool, caFile, certFile, keyFile string, err error) {
 	if registry.IsOCI(repoURL) {
 		return fmt.Sprintf("%s/%s:%s", repoURL, name, version), "", "", false, false, "", "", "", nil
 	}
 
+	// First, try to look up the repository by name. This ensures correct
+	// credentials are used when multiple repositories share the same URL.
+	if repoName != "" {
+		if cr, ok := repos[repoName]; ok {
+			var entry repo.ChartVersions
+			entry, err = findEntryByName(name, cr)
+			if err == nil {
+				var ve *repo.ChartVersion
+				ve, err = findVersionedEntry(version, entry)
+				if err == nil {
+					url, err = repo.ResolveReferenceURL(repoURL, ve.URLs[0])
+					if err == nil {
+						username = cr.Config.Username
+						password = cr.Config.Password
+						passCredentialsAll = cr.Config.PassCredentialsAll
+						insecureSkipTLSVerify = cr.Config.InsecureSkipTLSVerify
+						caFile = cr.Config.CAFile
+						certFile = cr.Config.CertFile
+						keyFile = cr.Config.KeyFile
+						return
+					}
+				}
+			}
+		}
+	}
+
+	// Fall back to scanning repos by URL for backward compatibility.
 	for _, cr := range repos {
 		if urlutil.Equal(repoURL, cr.Config.URL) {
 			var entry repo.ChartVersions

pkg/downloader/manager_test.go

@@ -71,7 +71,7 @@ func TestFindChartURL(t *testing.T) {
 	version := "0.1.0"
 	repoURL := "http://example.com/charts"
 
-	churl, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err := m.findChartURL(name, version, repoURL, repos)
+	churl, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err := m.findChartURL(name, version, repoURL, "", repos)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -96,7 +96,7 @@ func TestFindChartURL(t *testing.T) {
 	version = "1.2.3"
 	repoURL = "https://example-https-insecureskiptlsverify.com"
 
-	churl, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err = m.findChartURL(name, version, repoURL, repos)
+	churl, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err = m.findChartURL(name, version, repoURL, "", repos)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -121,7 +121,7 @@ func TestFindChartURL(t *testing.T) {
 	version = "1.2.3"
 	repoURL = "http://example.com/helm"
 
-	churl, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err = m.findChartURL(name, version, repoURL, repos)
+	churl, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err = m.findChartURL(name, version, repoURL, "", repos)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -143,6 +143,123 @@ func TestFindChartURL(t *testing.T) {
 	}
 }
 
+// TestFindChartURL_DuplicateRepoURL tests that when two repositories share the same
+// URL but have different aliases and credentials, findChartURL correctly returns the
+// credentials for the specified repo alias (name) rather than picking a random
+// matching entry by URL.
+func TestFindChartURL_DuplicateRepoURL(t *testing.T) {
+	dir := t.TempDir()
+
+	// Create a repositories.yaml with two entries sharing the same URL
+	reposConfig := `apiVersion: v1
+repositories:
+  - name: repo-alias-1
+    url: "http://example.com/charts"
+    username: "user1"
+    password: "pass1"
+    pass_credentials_all: true
+  - name: repo-alias-2
+    url: "http://example.com/charts"
+    username: "user2"
+    password: "pass2"
+`
+	repoConfigFile := filepath.Join(dir, "repositories.yaml")
+	if err := os.WriteFile(repoConfigFile, []byte(reposConfig), 0644); err != nil {
+		t.Fatal(err)
+	}
+
+	// Create an index file for each repo alias (same content since they share the same URL)
+	index := repo.NewIndexFile()
+	index.Entries["alpine"] = repo.ChartVersions{
+		{
+			Metadata: &chart.Metadata{
+				Name:    "alpine",
+				Version: "0.1.0",
+			},
+			URLs: []string{
+				"https://charts.helm.sh/stable/alpine-0.1.0.tgz",
+			},
+		},
+	}
+
+	// Write index files with the naming pattern expected by loadChartRepositories:
+	// <cache>/<repo-name>-index.yaml
+	for _, name := range []string{"repo-alias-1", "repo-alias-2"} {
+		idxPath := filepath.Join(dir, name+"-index.yaml")
+		if err := index.WriteFile(idxPath, 0644); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	m := &Manager{
+		Out:              new(bytes.Buffer),
+		RepositoryConfig: repoConfigFile,
+		RepositoryCache:  dir,
+	}
+
+	repos, err := m.loadChartRepositories()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	name := "alpine"
+	version := "0.1.0"
+	repoURL := "http://example.com/charts"
+
+	// Look up by repo-alias-1 name -> should get user1/pass1
+	churl, username, password, _, passCredentialsAll, _, _, _, err := m.findChartURL(name, version, repoURL, "repo-alias-1", repos)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if churl != "https://charts.helm.sh/stable/alpine-0.1.0.tgz" {
+		t.Errorf("Unexpected URL %q", churl)
+	}
+	if username != "user1" {
+		t.Errorf("Expected username 'user1', got %q", username)
+	}
+	if password != "pass1" {
+		t.Errorf("Expected password 'pass1', got %q", password)
+	}
+	if !passCredentialsAll {
+		t.Errorf("Expected passCredentialsAll true, got %t", passCredentialsAll)
+	}
+
+	// Look up by repo-alias-2 name -> should get user2/pass2
+	churl, username, password, _, passCredentialsAll, _, _, _, err = m.findChartURL(name, version, repoURL, "repo-alias-2", repos)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if churl != "https://charts.helm.sh/stable/alpine-0.1.0.tgz" {
+		t.Errorf("Unexpected URL %q", churl)
+	}
+	if username != "user2" {
+		t.Errorf("Expected username 'user2', got %q", username)
+	}
+	if password != "pass2" {
+		t.Errorf("Expected password 'pass2', got %q", password)
+	}
+	// passCredentialsAll should be false for repo-alias-2 (not set in config)
+	if passCredentialsAll {
+		t.Errorf("Expected passCredentialsAll false, got %t", passCredentialsAll)
+	}
+
+	// Without a repo name (empty string), falls back to URL-based scan.
+	// This is backward-compatible behavior.
+	churl, username, password, _, _, _, _, _, err = m.findChartURL(name, version, repoURL, "", repos)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if churl != "https://charts.helm.sh/stable/alpine-0.1.0.tgz" {
+		t.Errorf("Unexpected URL %q", churl)
+	}
+	// Without a repo name, it falls back to URL-based matching.
+	// Either repo's credentials could be returned since map iteration is nondeterministic.
+	// We just verify that some credentials were returned.
+	if username == "" {
+		t.Error("Expected non-empty username from URL-based fallback")
+	}
+}
+
 func TestGetRepoNames(t *testing.T) {
 	b := bytes.NewBuffer(nil)
 	m := &Manager{
@@ -267,7 +384,7 @@ func TestDownloadAll(t *testing.T) {
 	if err := os.MkdirAll(filepath.Join(chartPath, "tmpcharts"), 0755); err != nil {
 		t.Fatal(err)
 	}
-	if err := m.downloadAll([]*chart.Dependency{signDep, localDep}); err != nil {
+	if err := m.downloadAll([]*chart.Dependency{signDep, localDep}, nil); err != nil {
 		t.Error(err)
 	}
 
@@ -296,7 +413,7 @@ version: 0.1.0`
 		Version:    "0.1.0",
 	}
 
-	err = m.downloadAll([]*chart.Dependency{badLocalDep})
+	err = m.downloadAll([]*chart.Dependency{badLocalDep}, nil)
 	if err == nil {
 		t.Fatal("Expected error for bad dependency name")
 	}