fix(git): pick the credential for the transport the remote actually uses

internal/adapters/secondary/git/git.go

@@ -33,6 +33,23 @@ func UpdateCache(config env.ConfigProvider, dep domain.Dependency) (*goGit.Repos
 	return UpdateCacheNative(dep)
 }
 
+// remoteURL returns the URL the repository actually fetches from.
+//
+// This is not always dep.GetURL(): a cache cloned before an SSH login was
+// configured for the host keeps its original HTTPS remote, and the credential
+// has to be picked for the transport in use, not for the one the manifest
+// would produce today.
+func remoteURL(repository *goGit.Repository, dep domain.Dependency) string {
+	if repository != nil {
+		if remote, err := repository.Remote(goGit.DefaultRemoteName); err == nil {
+			if urls := remote.Config().URLs; len(urls) > 0 {
+				return urls[0]
+			}
+		}
+	}
+	return dep.GetURL()
+}
+
 func initSubmodules(config env.ConfigProvider, dep domain.Dependency, repository *goGit.Repository) error {
 	worktree, err := repository.Worktree()
 	if err != nil {
@@ -46,7 +63,7 @@ func initSubmodules(config env.ConfigProvider, dep domain.Dependency, repository
 	err = submodules.Update(&goGit.SubmoduleUpdateOptions{
 		Init:              true,
 		RecurseSubmodules: goGit.DefaultSubmoduleRecursionDepth,
-		Auth:              config.GetAuth(dep.GetURLPrefix()),
+		Auth:              config.GetAuthForURL(dep.GetURLPrefix(), remoteURL(repository, dep)),
 	})
 	if err != nil {
 		return err
@@ -70,7 +87,7 @@ func GetVersions(config env.ConfigProvider, repository *goGit.Repository, dep do
 	err := repository.Fetch(&goGit.FetchOptions{
 		Force: true,
 		Prune: true,
-		Auth:  config.GetAuth(dep.GetURLPrefix()),
+		Auth:  config.GetAuthForURL(dep.GetURLPrefix(), remoteURL(repository, dep)),
 		RefSpecs: []gitConfig.RefSpec{
 			"refs/*:refs/*",
 			"HEAD:refs/heads/HEAD",

internal/adapters/secondary/git/git_embedded.go

@@ -26,7 +26,7 @@ func CloneCacheEmbedded(config env.ConfigProvider, dep domain.Dependency) (*git.
 	storageCache := makeStorageCache(config, dep)
 	worktreeFileSystem := createWorktreeFs(config, dep)
 	url := dep.GetURL()
-	auth := config.GetAuth(dep.GetURLPrefix())
+	auth := config.GetAuthForURL(dep.GetURLPrefix(), url)
 
 	cloneOpts := &git.CloneOptions{
 		URL:  url,
@@ -73,9 +73,12 @@ func UpdateCacheEmbedded(config env.ConfigProvider, dep domain.Dependency) (*git
 
 	err = repository.Fetch(&git.FetchOptions{
 		Force: true,
-		Auth:  config.GetAuth(dep.GetURLPrefix())})
+		Auth:  config.GetAuthForURL(dep.GetURLPrefix(), remoteURL(repository, dep))})
 	if err != nil && err.Error() != "already up-to-date" {
-		msg.Debug("Error to fetch repository of %s: %s", dep.Repository, err)
+		// The cached copy is still usable, so this is not fatal -- but it is the
+		// difference between "up to date" and "whatever was cached last time",
+		// which the user has to be able to see.
+		msg.Warn("⚠️ Could not update %s, using the cached copy: %s", dep.Repository, err)
 	}
 	if err := initSubmodules(config, dep, repository); err != nil {
 		return nil, err
@@ -133,6 +136,6 @@ func PullEmbedded(config env.ConfigProvider, dep domain.Dependency) error {
 	}
 	return worktree.Pull(&git.PullOptions{
 		Force: true,
-		Auth:  config.GetAuth(dep.GetURLPrefix()),
+		Auth:  config.GetAuthForURL(dep.GetURLPrefix(), remoteURL(repository, dep)),
 	})
 }

pkg/env/auth_transport_test.go

@@ -0,0 +1,143 @@
+package env_test
+
+import (
+	"crypto/ed25519"
+	"crypto/rand"
+	"encoding/pem"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/go-git/go-git/v5/plumbing/transport/http"
+	"github.com/hashload/boss/pkg/env"
+	"golang.org/x/crypto/ssh"
+)
+
+// A cache cloned over HTTPS keeps its HTTPS remote even after `boss login -s`
+// configures SSH for the host. Handing the SSH credential to the HTTP transport
+// makes go-git reject the fetch with "invalid auth method", so every dependency
+// of that host silently stops updating.
+func TestGetAuthForURLDropsSSHCredentialOnHTTPRemote(t *testing.T) {
+	config := newConfigWithAuth(t, &env.Auth{UseSSH: true, Path: "/home/user/.ssh/id_ed25519"})
+
+	if got := config.GetAuthForURL("github.com", "https://github.com/hashload/horse"); got != nil {
+		t.Errorf("GetAuthForURL() = %v, want nil for an SSH credential on an HTTPS remote", got)
+	}
+}
+
+func TestGetAuthForURLDropsBasicCredentialOnSSHRemote(t *testing.T) {
+	auth := &env.Auth{}
+	auth.SetUser("octocat")
+	auth.SetPass("s3cr3t")
+	config := newConfigWithAuth(t, auth)
+
+	if got := config.GetAuthForURL("github.com", "git@github.com:hashload/horse"); got != nil {
+		t.Errorf("GetAuthForURL() = %v, want nil for a basic credential on an SSH remote", got)
+	}
+}
+
+func TestGetAuthForURLKeepsMatchingCredential(t *testing.T) {
+	auth := &env.Auth{}
+	auth.SetUser("octocat")
+	auth.SetPass("s3cr3t")
+	config := newConfigWithAuth(t, auth)
+
+	got := config.GetAuthForURL("github.com", "https://github.com/hashload/horse")
+	basic, ok := got.(*http.BasicAuth)
+	if !ok {
+		t.Fatalf("GetAuthForURL() = %T, want *http.BasicAuth", got)
+	}
+	if basic.Username != "octocat" || basic.Password != "s3cr3t" {
+		t.Errorf("GetAuthForURL() returned %q/%q, want octocat/s3cr3t", basic.Username, basic.Password)
+	}
+}
+
+func TestGetAuthForURLWithoutStoredCredential(t *testing.T) {
+	config := newConfigWithAuth(t, nil)
+
+	if got := config.GetAuthForURL("gitlab.com", "https://gitlab.com/group/project"); got != nil {
+		t.Errorf("GetAuthForURL() = %v, want nil when nothing is stored for the host", got)
+	}
+}
+
+// An empty URL means the caller could not determine the transport. Falling back
+// to the stored credential keeps the previous behaviour rather than silently
+// dropping authentication.
+func TestGetAuthForURLFallsBackWhenURLIsUnknown(t *testing.T) {
+	auth := &env.Auth{}
+	auth.SetUser("octocat")
+	auth.SetPass("s3cr3t")
+	config := newConfigWithAuth(t, auth)
+
+	if got := config.GetAuthForURL("github.com", ""); got == nil {
+		t.Error("GetAuthForURL() = nil, want the stored credential when the URL is unknown")
+	}
+}
+
+func TestGetAuthForURLTransportDetection(t *testing.T) {
+	sshAuth := &env.Auth{UseSSH: true, Path: writeTestSSHKey(t)}
+	sshAuth.SetPassPhrase(testKeyPassphrase)
+	config := newConfigWithAuth(t, sshAuth)
+
+	sshRemotes := []string{
+		"git@github.com:hashload/horse",
+		"ssh://git@github.com/hashload/horse",
+		"git@mygitlab.domain.de:delphi/libraries/mylib.git",
+	}
+	for _, remote := range sshRemotes {
+		if got := config.GetAuthForURL("github.com", remote); got == nil {
+			t.Errorf("GetAuthForURL(%q) = nil, want the SSH credential", remote)
+		}
+	}
+
+	httpRemotes := []string{
+		"https://github.com/hashload/horse",
+		"http://github.com/hashload/horse",
+		// A user in an HTTPS URL must not be mistaken for scp-like syntax.
+		"https://octocat@github.com/hashload/horse",
+	}
+	for _, remote := range httpRemotes {
+		if got := config.GetAuthForURL("github.com", remote); got != nil {
+			t.Errorf("GetAuthForURL(%q) = %v, want nil", remote, got)
+		}
+	}
+}
+
+// testKeyPassphrase protects the generated key. Using an encrypted key keeps
+// this test independent of how an empty passphrase is handled.
+const testKeyPassphrase = "test-passphrase"
+
+// writeTestSSHKey writes an encrypted ed25519 key and returns its path.
+// The SSH branch of GetAuth parses the key file, so it needs a real one.
+func writeTestSSHKey(t *testing.T) string {
+	t.Helper()
+
+	_, private, err := ed25519.GenerateKey(rand.Reader)
+	if err != nil {
+		t.Fatalf("Failed to generate the test key: %v", err)
+	}
+
+	block, err := ssh.MarshalPrivateKeyWithPassphrase(private, "", []byte(testKeyPassphrase))
+	if err != nil {
+		t.Fatalf("Failed to marshal the test key: %v", err)
+	}
+
+	path := filepath.Join(t.TempDir(), "id_ed25519")
+	if err := os.WriteFile(path, pem.EncodeToMemory(block), 0600); err != nil {
+		t.Fatalf("Failed to write the test key: %v", err)
+	}
+	return path
+}
+
+func newConfigWithAuth(t *testing.T, auth *env.Auth) *env.Configuration {
+	t.Helper()
+
+	config, err := env.LoadConfiguration(t.TempDir())
+	if config == nil {
+		t.Fatalf("LoadConfiguration() returned no configuration: %v", err)
+	}
+	if auth != nil {
+		config.Auth["github.com"] = auth
+	}
+	return config
+}

pkg/env/configuration.go

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"os"
 	"path/filepath"
+	"strings"
 	"time"
 
 	"github.com/go-git/go-git/v5/plumbing/transport"
@@ -127,7 +128,53 @@ func (a *Auth) SetPassPhrase(passphrase string) {
 	}
 }
 
+// usesSSHTransport reports whether rawURL is fetched over SSH.
+func usesSSHTransport(rawURL string) bool {
+	switch {
+	case strings.HasPrefix(rawURL, "ssh://"):
+		return true
+	case strings.HasPrefix(rawURL, "http://"),
+		strings.HasPrefix(rawURL, "https://"),
+		strings.HasPrefix(rawURL, "git://"),
+		strings.HasPrefix(rawURL, "file://"):
+		return false
+	default:
+		// scp-like syntax: [user@]host:path
+		return strings.Contains(rawURL, "@")
+	}
+}
+
+// GetAuthForURL returns the authentication method for a repository, but only
+// when the stored credential fits the transport rawURL will use.
+//
+// The credential is keyed by host, while the transport comes from the URL the
+// repository actually fetches from. Those two disagree whenever a cache was
+// cloned over HTTPS before an SSH login was configured for that host. Handing
+// an SSH credential to the HTTP transport makes go-git fail the fetch with
+// "invalid auth method" -- older versions silently ignored it instead, which is
+// why this only started biting after the go-git upgrade.
+//
+// Returning nil means "fetch anonymously", which is what go-git used to do on
+// its own and is correct for the public repositories this affects. A private
+// repository reached over the wrong transport still fails, as it always did.
+func (c *Configuration) GetAuthForURL(repo, rawURL string) transport.AuthMethod {
+	auth := c.Auth[repo]
+	if auth == nil {
+		return nil
+	}
+
+	if rawURL != "" && auth.UseSSH != usesSSHTransport(rawURL) {
+		msg.Debug("Skipping the credential stored for %s: it does not fit the transport of %s", repo, rawURL)
+		return nil
+	}
+
+	return c.GetAuth(repo)
+}
+
 // GetAuth returns the authentication method for a repository.
+//
+// Prefer GetAuthForURL when the URL being reached is known: this one cannot
+// tell whether the credential fits the transport.
 func (c *Configuration) GetAuth(repo string) transport.AuthMethod {
 	auth := c.Auth[repo]
 

pkg/env/interfaces.go

@@ -12,6 +12,7 @@ type ConfigProvider interface {
 	GetDelphiPath() string
 	GetGitEmbedded() bool
 	GetAuth(repo string) transport.AuthMethod
+	GetAuthForURL(repo, rawURL string) transport.AuthMethod
 	GetPurgeTime() int
 	GetInternalRefreshRate() int
 	GetLastPurge() time.Time

pkg/env/legacy_auth_test.go

@@ -73,8 +73,8 @@ func TestLegacyAuthFieldsSurviveLoadAndSave(t *testing.T) {
 	}
 
 	configPath := filepath.Join(tempDir, consts.BossConfigFile)
-	if err := os.WriteFile(configPath, data, 0600); err != nil {
-		t.Fatalf("Failed to write config file: %v", err)
+	if writeErr := os.WriteFile(configPath, data, 0600); writeErr != nil {
+		t.Fatalf("Failed to write config file: %v", writeErr)
 	}
 
 	config, err := env.LoadConfiguration(tempDir)

setup/migrations.go

@@ -61,43 +61,57 @@ func seven() {
 	migrated := false
 
 	for repo, auth := range configuration.Auth {
-		if auth == nil {
-			continue
+		if migrateLegacyAuth(repo, auth) {
+			migrated = true
 		}
+	}
 
-		if auth.LegacyUser != "" {
-			if decrypted, err := oldDecrypt(auth.LegacyUser); err != nil {
-				msg.Warn("⚠️ Migration 7: could not migrate the user for %s: %v", repo, err)
-			} else {
-				auth.SetUser(decrypted)
-			}
-		}
+	if migrated {
+		configuration.SaveConfiguration()
+	}
+}
 
-		if auth.LegacyPass != "" {
-			if decrypted, err := oldDecrypt(auth.LegacyPass); err != nil {
-				msg.Warn("⚠️ Migration 7: could not migrate the password for %s: %v", repo, err)
-			} else {
-				auth.SetPass(decrypted)
-			}
-		}
+// migrateLegacyAuth converts one entry's legacy credentials and clears them,
+// reporting whether anything was converted.
+func migrateLegacyAuth(repo string, auth *env.Auth) bool {
+	if auth == nil {
+		return false
+	}
+	if auth.LegacyUser == "" && auth.LegacyPass == "" && auth.LegacyPassPhrase == "" {
+		return false
+	}
 
-		if auth.LegacyPassPhrase != "" {
-			if decrypted, err := oldDecrypt(auth.LegacyPassPhrase); err != nil {
-				msg.Warn("⚠️ Migration 7: could not migrate the passphrase for %s: %v", repo, err)
-			} else if decrypted != "" {
-				auth.SetPassPhrase(decrypted)
-			}
-		}
+	if decrypted, ok := decryptLegacy(repo, "user", auth.LegacyUser); ok {
+		auth.SetUser(decrypted)
+	}
+	if decrypted, ok := decryptLegacy(repo, "password", auth.LegacyPass); ok {
+		auth.SetPass(decrypted)
+	}
+	// An empty passphrase means the key has none, and storing it back would only
+	// re-create the value migration 7 exists to retire.
+	if decrypted, ok := decryptLegacy(repo, "passphrase", auth.LegacyPassPhrase); ok && decrypted != "" {
+		auth.SetPassPhrase(decrypted)
+	}
 
-		if auth.LegacyUser != "" || auth.LegacyPass != "" || auth.LegacyPassPhrase != "" {
-			auth.LegacyUser, auth.LegacyPass, auth.LegacyPassPhrase = "", "", ""
-			migrated = true
-		}
+	auth.LegacyUser, auth.LegacyPass, auth.LegacyPassPhrase = "", "", ""
+
+	return true
+}
+
+// decryptLegacy decrypts one legacy value, warning instead of aborting when it
+// cannot be read. It reports false when there was nothing to convert.
+func decryptLegacy(repo, field, value string) (string, bool) {
+	if value == "" {
+		return "", false
 	}
 
-	if migrated {
-		configuration.SaveConfiguration()
+	decrypted, err := oldDecrypt(value)
+	if err != nil {
+		msg.Warn("⚠️ Migration 7: could not migrate the %s for %s: %v", field, repo, err)
+		return "", false
 	}
+
+	return decrypted, true
 }
 
 // cleanup cleans up the internal global directory.