diff --git a/internal/adapters/secondary/git/git.go b/internal/adapters/secondary/git/git.go index 952464e..44e8c9c 100644 --- a/internal/adapters/secondary/git/git.go +++ b/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", diff --git a/internal/adapters/secondary/git/git_embedded.go b/internal/adapters/secondary/git/git_embedded.go index 95b5a5b..5434ffd 100644 --- a/internal/adapters/secondary/git/git_embedded.go +++ b/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)), }) } diff --git a/pkg/env/auth_transport_test.go b/pkg/env/auth_transport_test.go new file mode 100644 index 0000000..da7922a --- /dev/null +++ b/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 +} diff --git a/pkg/env/configuration.go b/pkg/env/configuration.go index 5b5dc55..687db5d 100644 --- a/pkg/env/configuration.go +++ b/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] diff --git a/pkg/env/interfaces.go b/pkg/env/interfaces.go index 653b9b5..bf1c621 100644 --- a/pkg/env/interfaces.go +++ b/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 diff --git a/pkg/env/legacy_auth_test.go b/pkg/env/legacy_auth_test.go index 725f646..2f965b5 100644 --- a/pkg/env/legacy_auth_test.go +++ b/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) diff --git a/setup/migrations.go b/setup/migrations.go index 2137821..68748e1 100644 --- a/setup/migrations.go +++ b/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.