From 78e7d8e4ee287dbb66d80085799a1f94dbe2903b Mon Sep 17 00:00:00 2001 From: Dominik Schulz Date: Fri, 29 Mar 2024 15:29:51 +0100 Subject: [PATCH] [bugfix] Fix loading of git configs (#2849) * [bugfix] Fix loading of git configs The gitconfig package was incorrectly using gopass specific locations when trying to load global (per user) git configs. This change makes it use the correct locations. Fixes #2686 Signed-off-by: Dominik Schulz * Fix typo Signed-off-by: Dominik Schulz * Fix linter issues Signed-off-by: Dominik Schulz * Fix failing tests Those started to fail because we are now correctly reading global git configs. Signed-off-by: Dominik Schulz --------- Signed-off-by: Dominik Schulz --- internal/action/init.go | 5 ++- internal/config/config.go | 1 + pkg/appdir/appdir.go | 36 +++++++++++++++++++-- pkg/appdir/appdir_windows.go | 18 +++++------ pkg/appdir/appdir_xdg.go | 18 +++++------ pkg/gitconfig/config.go | 18 ++++++----- pkg/gitconfig/configs.go | 50 ++++++++++++++++-------------- pkg/gitconfig/gitconfig.go | 4 +-- pkg/gitconfig/gitconfig_others.go | 7 +++++ pkg/gitconfig/gitconfig_windows.go | 27 ++++++++++++++++ pkg/termio/identity_test.go | 43 +++++++++---------------- 11 files changed, 142 insertions(+), 85 deletions(-) create mode 100644 pkg/gitconfig/gitconfig_others.go create mode 100644 pkg/gitconfig/gitconfig_windows.go diff --git a/internal/action/init.go b/internal/action/init.go index 5ba8d3802e..f1d87cb96e 100644 --- a/internal/action/init.go +++ b/internal/action/init.go @@ -179,11 +179,10 @@ func (s *Action) init(ctx context.Context, alias, path string, keys ...string) e func (s *Action) printRecipients(ctx context.Context, alias string) { crypto := s.Store.Crypto(ctx, alias) for _, recipient := range s.Store.ListRecipients(ctx, alias) { - r := "0x" + recipient if kl, err := crypto.FindRecipients(ctx, recipient); err == nil && len(kl) > 0 { - r = crypto.FormatKey(ctx, kl[0], "") + recipient = crypto.FormatKey(ctx, kl[0], "") } - out.Printf(ctx, "📩 "+r) + out.Printf(ctx, "📩 "+recipient) } } diff --git a/internal/config/config.go b/internal/config/config.go index b256d5a4e7..c55d3fc4d2 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -35,6 +35,7 @@ const ( func newGitconfig() *gitconfig.Configs { c := gitconfig.New() + c.Name = "gopass" c.EnvPrefix = envPrefix c.GlobalConfig = os.Getenv("GOPASS_CONFIG") c.SystemConfig = systemConfig diff --git a/pkg/appdir/appdir.go b/pkg/appdir/appdir.go index 65f07a421d..23aea4cf10 100644 --- a/pkg/appdir/appdir.go +++ b/pkg/appdir/appdir.go @@ -9,8 +9,40 @@ import ( "github.com/gopasspw/gopass/pkg/debug" ) -// Name is used in the final path of the generated path. -var Name = "gopass" +var DefaultAppdir = New("gopass") + +// Appdir is a helper struct to generate paths for config, cache and data dirs. +type Appdir struct { + // Name is used in the final path of the generated path. + name string +} + +// New returns a new Appdir. +func New(name string) *Appdir { + return &Appdir{ + name: name, + } +} + +// Name returns the name of the appdir. +func (a *Appdir) Name() string { + return a.name +} + +// UserConfig returns the users config dir. +func UserConfig() string { + return DefaultAppdir.UserConfig() +} + +// UserCache returns the users cache dir. +func UserCache() string { + return DefaultAppdir.UserCache() +} + +// UserData returns the users data dir. +func UserData() string { + return DefaultAppdir.UserData() +} // UserHome returns the users home dir. func UserHome() string { diff --git a/pkg/appdir/appdir_windows.go b/pkg/appdir/appdir_windows.go index ee5c28a099..a87cc061e2 100644 --- a/pkg/appdir/appdir_windows.go +++ b/pkg/appdir/appdir_windows.go @@ -6,27 +6,27 @@ import ( ) // UserConfig returns the users config dir -func UserConfig() string { +func (a *Appdir) UserConfig() string { if hd := os.Getenv("GOPASS_HOMEDIR"); hd != "" { - return filepath.Join(hd, ".config", Name) + return filepath.Join(hd, ".config", a.name) } - return filepath.Join(os.Getenv("APPDATA"), Name) + return filepath.Join(os.Getenv("APPDATA"), a.name) } // UserCache returns the users cache dir -func UserCache() string { +func (a *Appdir) UserCache() string { if hd := os.Getenv("GOPASS_HOMEDIR"); hd != "" { - return filepath.Join(hd, ".cache", Name) + return filepath.Join(hd, ".cache", a.name) } - return filepath.Join(os.Getenv("LOCALAPPDATA"), Name) + return filepath.Join(os.Getenv("LOCALAPPDATA"), a.name) } // UserData returns the users data dir -func UserData() string { +func (a *Appdir) UserData() string { if hd := os.Getenv("GOPASS_HOMEDIR"); hd != "" { - return filepath.Join(hd, ".local", "share", Name) + return filepath.Join(hd, ".local", "share", a.name) } - return filepath.Join(os.Getenv("LOCALAPPDATA"), Name) + return filepath.Join(os.Getenv("LOCALAPPDATA"), a.name) } diff --git a/pkg/appdir/appdir_xdg.go b/pkg/appdir/appdir_xdg.go index 9093ee15bc..2745f89b2a 100644 --- a/pkg/appdir/appdir_xdg.go +++ b/pkg/appdir/appdir_xdg.go @@ -9,9 +9,9 @@ import ( ) // UserConfig returns the users config dir. -func UserConfig() string { +func (a *Appdir) UserConfig() string { if hd := os.Getenv("GOPASS_HOMEDIR"); hd != "" { - return filepath.Join(hd, ".config", Name) + return filepath.Join(hd, ".config", a.name) } base := os.Getenv("XDG_CONFIG_HOME") @@ -19,13 +19,13 @@ func UserConfig() string { base = filepath.Join(os.Getenv("HOME"), ".config") } - return filepath.Join(base, Name) + return filepath.Join(base, a.name) } // UserCache returns the users cache dir. -func UserCache() string { +func (a *Appdir) UserCache() string { if hd := os.Getenv("GOPASS_HOMEDIR"); hd != "" { - return filepath.Join(hd, ".cache", Name) + return filepath.Join(hd, ".cache", a.name) } base := os.Getenv("XDG_CACHE_HOME") @@ -33,13 +33,13 @@ func UserCache() string { base = filepath.Join(os.Getenv("HOME"), ".cache") } - return filepath.Join(base, Name) + return filepath.Join(base, a.name) } // UserData returns the users data dir. -func UserData() string { +func (a *Appdir) UserData() string { if hd := os.Getenv("GOPASS_HOMEDIR"); hd != "" { - return filepath.Join(hd, ".local", "share", Name) + return filepath.Join(hd, ".local", "share", a.name) } base := os.Getenv("XDG_DATA_HOME") @@ -47,5 +47,5 @@ func UserData() string { base = filepath.Join(os.Getenv("HOME"), ".local", "share") } - return filepath.Join(base, Name) + return filepath.Join(base, a.name) } diff --git a/pkg/gitconfig/config.go b/pkg/gitconfig/config.go index 6beb9824b0..5beccbe568 100644 --- a/pkg/gitconfig/config.go +++ b/pkg/gitconfig/config.go @@ -122,16 +122,16 @@ func (c *Config) Set(key, value string) error { vs[0] = value c.vars[key] = vs - debug.Log("set %q to %q", key, value) + // debug.Log("set %q to %q", key, value) // a new key, insert it into an existing section, if any if !present { - debug.Log("inserting value") + // debug.Log("inserting value") return c.insertValue(key, value) } - debug.Log("updating value") + // debug.Log("updating value") var updated bool @@ -146,7 +146,7 @@ func (c *Config) Set(key, value string) error { } func (c *Config) insertValue(key, value string) error { - debug.Log("input (%s: %s): \n--------------\n%s\n--------------\n", key, value, strings.Join(strings.Split("- "+c.raw.String(), "\n"), "\n- ")) + // debug.Log("input (%s: %s): \n--------------\n%s\n--------------\n", key, value, strings.Join(strings.Split("- "+c.raw.String(), "\n"), "\n- ")) wSection, wSubsection, wKey := splitKey(key) @@ -204,7 +204,7 @@ func (c *Config) insertValue(key, value string) error { c.raw.WriteString(strings.Join(lines, "\n")) c.raw.WriteString("\n") - debug.Log("output: \n--------------\n%s\n--------------\n", strings.Join(strings.Split("+ "+c.raw.String(), "\n"), "\n+ ")) + // debug.Log("output: \n--------------\n%s\n--------------\n", strings.Join(strings.Split("+ "+c.raw.String(), "\n"), "\n+ ")) return c.flushRaw() } @@ -231,7 +231,7 @@ func parseSectionHeader(line string) (section, subsection string, skip bool) { / // rewriteRaw is used to rewrite the raw config copy. It is used for set and unset operations // with different callbacks each. func (c *Config) rewriteRaw(key, value string, cb parseFunc) error { - debug.Log("input (%s: %s): \n--------------\n%s\n--------------\n", key, value, strings.Join(strings.Split("- "+c.raw.String(), "\n"), "\n- ")) + // debug.Log("input (%s: %s): \n--------------\n%s\n--------------\n", key, value, strings.Join(strings.Split("- "+c.raw.String(), "\n"), "\n- ")) lines := parseConfig(strings.NewReader(c.raw.String()), key, value, cb) @@ -239,7 +239,7 @@ func (c *Config) rewriteRaw(key, value string, cb parseFunc) error { c.raw.WriteString(strings.Join(lines, "\n")) c.raw.WriteString("\n") - debug.Log("output: \n--------------\n%s\n--------------\n", strings.Join(strings.Split("+ "+c.raw.String(), "\n"), "\n+ ")) + // debug.Log("output: \n--------------\n%s\n--------------\n", strings.Join(strings.Split("+ "+c.raw.String(), "\n"), "\n+ ")) return c.flushRaw() } @@ -255,12 +255,14 @@ func (c *Config) flushRaw() error { return err } - debug.Log("writing config to %s: \n--------------\n%s\n--------------", c.path, c.raw.String()) + // debug.Log("writing config to %s: \n--------------\n%s\n--------------", c.path, c.raw.String()) if err := os.WriteFile(c.path, []byte(c.raw.String()), 0o600); err != nil { return fmt.Errorf("failed to write config to %s: %w", c.path, err) } + debug.Log("wrote config to %s", c.path) + return nil } diff --git a/pkg/gitconfig/configs.go b/pkg/gitconfig/configs.go index 6069329b3b..817c72fb7b 100644 --- a/pkg/gitconfig/configs.go +++ b/pkg/gitconfig/configs.go @@ -23,6 +23,7 @@ type Configs struct { env *Config workdir string + Name string SystemConfig string GlobalConfig string LocalConfig string @@ -37,7 +38,7 @@ func New() *Configs { readonly: true, }, global: &Config{ - path: globalConfigFile(), + path: globalConfigFile(name), }, local: &Config{}, worktree: &Config{}, @@ -45,6 +46,7 @@ func New() *Configs { noWrites: true, }, + Name: name, SystemConfig: systemConfig, GlobalConfig: globalConfig, LocalConfig: localConfig, @@ -60,7 +62,7 @@ func (cs *Configs) Reload() { // String implements fmt.Stringer. func (cs *Configs) String() string { - return fmt.Sprintf("GitConfigs{Workdir: %s - Env: %s - System: %s - Global: %s - Local: %s - Worktree: %s}", cs.workdir, cs.EnvPrefix, cs.SystemConfig, cs.GlobalConfig, cs.LocalConfig, cs.WorktreeConfig) + return fmt.Sprintf("GitConfigs{Name: %s - Workdir: %s - Env: %s - System: %s - Global: %s - Local: %s - Worktree: %s}", cs.Name, cs.workdir, cs.EnvPrefix, cs.SystemConfig, cs.GlobalConfig, cs.LocalConfig, cs.WorktreeConfig) } // LoadAll tries to load all known config files. Missing or invalid files are @@ -69,15 +71,15 @@ func (cs *Configs) String() string { func (cs *Configs) LoadAll(workdir string) *Configs { cs.workdir = workdir - debug.Log("Loading gitconfigs for %s", cs) + debug.Log("Loading gitconfigs for %s", cs.Name) // load the system config, if any if os.Getenv(cs.EnvPrefix+"_NOSYSTEM") == "" { c, err := LoadConfig(cs.SystemConfig) if err != nil { - debug.Log("[%s] failed to load system config: %s", cs.EnvPrefix, err) + debug.Log("[%s] failed to load system config: %s", cs.Name, err) } else { - debug.Log("[%s] loaded system config from %s", cs.EnvPrefix, cs.SystemConfig) + debug.Log("[%s] loaded system config from %s", cs.Name, cs.SystemConfig) cs.system = c // the system config should generally not be written from gopass. // in almost any scenario gopass shouldn't have write access @@ -96,11 +98,11 @@ func (cs *Configs) LoadAll(workdir string) *Configs { localConfigPath := filepath.Join(workdir, cs.LocalConfig) c, err := LoadConfig(localConfigPath) if err != nil { - debug.Log("[%s] failed to load local config from %s: %s", cs.EnvPrefix, localConfigPath, err) + debug.Log("[%s] failed to load local config from %s: %s", cs.Name, localConfigPath, err) // set the path just in case we want to modify / write to it later cs.local.path = localConfigPath } else { - debug.Log("[%s] loaded local config from %s", cs.EnvPrefix, localConfigPath) + debug.Log("[%s] loaded local config from %s", cs.Name, localConfigPath) cs.local = c } } @@ -111,11 +113,11 @@ func (cs *Configs) LoadAll(workdir string) *Configs { worktreeConfigPath := filepath.Join(workdir, cs.WorktreeConfig) c, err := LoadConfig(worktreeConfigPath) if err != nil { - debug.Log("[%s] failed to load worktree config from %s: %s", cs.EnvPrefix, worktreeConfigPath, err) + // debug.Log("[%s] failed to load worktree config from %s: %s", cs.Name, worktreeConfigPath, err) // set the path just in case we want to modify / write to it later cs.worktree.path = worktreeConfigPath } else { - debug.Log("[%s] loaded worktree config from %s", cs.EnvPrefix, worktreeConfigPath) + debug.Log("[%s] loaded worktree config from %s", cs.Name, worktreeConfigPath) cs.worktree = c } } @@ -127,9 +129,9 @@ func (cs *Configs) LoadAll(workdir string) *Configs { return cs } -func globalConfigFile() string { +func globalConfigFile(name string) string { // $XDG_CONFIG_HOME/git/config - return filepath.Join(appdir.UserConfig(), "config") + return filepath.Join(appdir.New(name).UserConfig(), "config") } // loadGlobalConfigs will try to load the per-user (Git calls them "global") configs. @@ -137,7 +139,7 @@ func globalConfigFile() string { // it's easier to handle this in its own method. func (cs *Configs) loadGlobalConfigs() string { locs := []string{ - globalConfigFile(), + globalConfigFile(cs.Name), } if cs.GlobalConfig != "" { @@ -148,10 +150,10 @@ func (cs *Configs) loadGlobalConfigs() string { // if we already have a global config we can just reload it instead of trying all locations if !cs.global.IsEmpty() { if p := cs.global.path; p != "" { - debug.Log("[%s] reloading existing global config from %s", cs.EnvPrefix, p) + debug.Log("[%s] reloading existing global config from %s", cs.Name, p) cfg, err := LoadConfig(p) if err != nil { - debug.Log("[%s] failed to reload global config from %s", cs.EnvPrefix, p) + debug.Log("[%s] failed to reload global config from %s", cs.Name, p) } else { cs.global = cfg @@ -160,7 +162,7 @@ func (cs *Configs) loadGlobalConfigs() string { } } - debug.Log("[%s] trying to find global configs in %v", cs.EnvPrefix, locs) + debug.Log("[%s] trying to find global configs in %v", cs.Name, locs) for _, p := range locs { // GlobalConfig might be set to an empty string to disable it // and instead of the XDG_CONFIG_HOME path only. @@ -169,22 +171,22 @@ func (cs *Configs) loadGlobalConfigs() string { } cfg, err := LoadConfig(p) if err != nil { - debug.Log("[%s] failed to load global config from %s", cs.EnvPrefix, p) + debug.Log("[%s] failed to load global config from %s: %s", cs.Name, p, err) continue } - debug.Log("[%s] loaded global config from %s", cs.EnvPrefix, p) + debug.Log("[%s] loaded global config from %s", cs.Name, p) cs.global = cfg return p } - debug.Log("[%s] no global config found", cs.EnvPrefix) + debug.Log("[%s] no global config found", cs.Name) // set the path to the default one in case we want to write to it (create it) later cs.global = &Config{ - path: globalConfigFile(), + path: globalConfigFile(cs.Name), } return "" @@ -214,7 +216,7 @@ func (cs *Configs) Get(key string) string { } } - debug.Log("[%s] no value for %s found", cs.EnvPrefix, key) + // debug.Log("[%s] no value for %s found", cs.Name, key) return "" } @@ -238,7 +240,7 @@ func (cs *Configs) GetAll(key string) []string { } } - debug.Log("[%s] no value for %s found", cs.EnvPrefix, key) + // debug.Log("[%s] no value for %s found", cs.Name, key) return nil } @@ -253,7 +255,7 @@ func (cs *Configs) GetGlobal(key string) string { return v } - debug.Log("[%s] no value for %s found", cs.EnvPrefix, key) + // debug.Log("[%s] no value for %s found", cs.Name, key) return "" } @@ -268,7 +270,7 @@ func (cs *Configs) GetLocal(key string) string { return v } - debug.Log("[%s] no value for %s found", cs.EnvPrefix, key) + // debug.Log("[%s] no value for %s found", cs.Name, key) return "" } @@ -309,7 +311,7 @@ func (cs *Configs) SetLocal(key, value string) error { func (cs *Configs) SetGlobal(key, value string) error { if cs.global == nil { cs.global = &Config{ - path: globalConfigFile(), + path: globalConfigFile(cs.Name), } } diff --git a/pkg/gitconfig/gitconfig.go b/pkg/gitconfig/gitconfig.go index 262c48bfd8..a79f5b06a4 100644 --- a/pkg/gitconfig/gitconfig.go +++ b/pkg/gitconfig/gitconfig.go @@ -1,8 +1,6 @@ package gitconfig var ( - // SystemConfig is the location of the (optional) system-wide config defaults file. - systemConfig = "/etc/gitconfig" // /etc/gopass/config // GlobalConfig is the location of the (optional) global (i.e. user-wide) config file. globalConfig = ".gitconfig" // LocalConfig is the name of the local (per-workdir) configuration. @@ -12,4 +10,6 @@ var ( worktreeConfig = "config.worktree" // EnvPrefix is the prefix for the environment variables controlling and overriding config variables. envPrefix = "GIT_CONFIG" + // Name is the name of the config directory (e.g. git or gopass). + name = "git" ) diff --git a/pkg/gitconfig/gitconfig_others.go b/pkg/gitconfig/gitconfig_others.go new file mode 100644 index 0000000000..447999708f --- /dev/null +++ b/pkg/gitconfig/gitconfig_others.go @@ -0,0 +1,7 @@ +//go:build !windows +// +build !windows + +package gitconfig + +// SystemConfig is the location of the (optional) system-wide config defaults file. +var systemConfig = "/etc/gitconfig" // /etc/gopass/config diff --git a/pkg/gitconfig/gitconfig_windows.go b/pkg/gitconfig/gitconfig_windows.go new file mode 100644 index 0000000000..603c035678 --- /dev/null +++ b/pkg/gitconfig/gitconfig_windows.go @@ -0,0 +1,27 @@ +//go:build windows +// +build windows + +package gitconfig + +import ( + "os/exec" + "path/filepath" + + "github.com/gopasspw/gopass/pkg/debug" +) + +var systemConfig string + +func init() { + gitPath, err := exec.LookPath("git.exe") + if err != nil { + debug.Log("git not found in PATH. Can not determine system config location") + + return + } + + // gitPath is something like C:\Program Files\Git\cmd\git.exe + // we need to strip the last two components to get the base path + // and then append etc/gitconfig. + systemConfig = filepath.Join(filepath.Dir(filepath.Dir(gitPath)), "etc", "gitconfig") +} diff --git a/pkg/termio/identity_test.go b/pkg/termio/identity_test.go index 6007516fb9..1541cb95f3 100644 --- a/pkg/termio/identity_test.go +++ b/pkg/termio/identity_test.go @@ -1,7 +1,6 @@ package termio import ( - "os" "testing" "github.com/gopasspw/gopass/internal/config" @@ -10,44 +9,32 @@ import ( func TestDetectName(t *testing.T) { ctx := config.NewContextInMemory() + td := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", td) + t.Setenv("GOPASS_HOMEDIR", td) - oga := os.Getenv("GIT_AUTHOR_NAME") - odf := os.Getenv("DEBFULLNAME") - ous := os.Getenv("USER") - - defer func() { - _ = os.Setenv("GIT_AUTHOR_NAME", oga) - _ = os.Setenv("DEBFULLNAME", odf) - _ = os.Setenv("USER", ous) - }() - - _ = os.Unsetenv("GIT_AUTHOR_NAME") - _ = os.Unsetenv("DEBFULLNAME") - _ = os.Unsetenv("USER") + t.Setenv("GIT_AUTHOR_NAME", "") + t.Setenv("DEBFULLNAME", "") + t.Setenv("USER", "") assert.Equal(t, "", DetectName(ctx, nil)) t.Setenv("USER", "foo") - assert.Equal(t, "foo", DetectName(ctx, nil)) } func TestDetectEmail(t *testing.T) { ctx := config.NewContextInMemory() + td := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", td) + t.Setenv("GOPASS_HOMEDIR", td) - oga := os.Getenv("GIT_AUTHOR_EMAIL") - odf := os.Getenv("DEBEMAIL") - ous := os.Getenv("EMAIL") - - defer func() { - _ = os.Setenv("GIT_AUTHOR_EMAIL", oga) - _ = os.Setenv("DEBEMAIL", odf) - _ = os.Setenv("EMAIL", ous) - }() - - _ = os.Unsetenv("GIT_AUTHOR_EMAIL") - _ = os.Unsetenv("DEBEMAIL") - _ = os.Unsetenv("EMAIL") + t.Setenv("GIT_AUTHOR_EMAIL", "") + t.Setenv("DEBEMAIL", "") + t.Setenv("EMAIL", "") assert.Equal(t, "", DetectEmail(ctx, nil)) + + t.Setenv("EMAIL", "foo@bar.de") + assert.Equal(t, "foo@bar.de", DetectEmail(ctx, nil)) }