From 036526a64bb7a887bcdf2911120ec7292fb419e7 Mon Sep 17 00:00:00 2001 From: Kai Armstrong Date: Wed, 22 Oct 2025 15:54:40 -0500 Subject: [PATCH] fix(config): implement XDG Base Directory Spec compliance This allows Linux distributions and system administrators to deploy site-wide default configurations that users can override, fixing the discrepancy between documented and actual XDG spec compliance. Fixes https://gitlab.com/gitlab-org/cli/-/issues/8018 --- README.md | 26 +++- go.mod | 1 + go.sum | 2 + internal/config/config_file.go | 71 ++++++++--- internal/config/config_file_test.go | 182 ++++++++++++++++++++++++++++ 5 files changed, 263 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 1712e5584..5c9056def 100644 --- a/README.md +++ b/README.md @@ -244,11 +244,18 @@ Endpoints allowing the use of the CI job token are listed in the ## Configuration By default, `glab` follows the -[XDG Base Directory Spec](https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html). -Configure it globally, locally, or per host: +[XDG Base Directory Spec](https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html), +which means it searches for configuration files in multiple locations with proper precedence. -- **Globally**: run `glab config set --global editor vim`. - - The global configuration file is available at `~/.config/glab-cli/config.yml`. +### Configuration Levels + +Configure `glab` at different levels: system-wide, globally (per-user), locally (per-repository), or per host: + +- **System-wide** (for all users): Place configuration at `/etc/xdg/glab-cli/config.yml` (or `$XDG_CONFIG_DIRS/glab-cli/config.yml`). + - Useful for Linux distributions and system administrators to provide default configurations. + - User configurations will override system-wide settings. +- **Globally** (per-user): run `glab config set --global editor vim`. + - The global configuration file is available at `~/.config/glab-cli/config.yml` (or `$XDG_CONFIG_HOME/glab-cli/config.yml`). - To override this location, set the `GLAB_CONFIG_DIR` environment variable. - **The current repository**: run `glab config set editor vim` in any folder in a Git repository. - The local configuration file is available at `.git/glab-cli/config.yml` in the current working Git directory. @@ -256,6 +263,17 @@ Configure it globally, locally, or per host: the `--host` parameter to meet your needs. - Per-host configuration info is always stored in the global configuration file, with or without the `global` flag. +### Configuration Search Order + +When `glab` looks for configuration files, it searches in this order (highest priority first): + +1. `$GLAB_CONFIG_DIR/config.yml` (if `GLAB_CONFIG_DIR` is set) +2. `$XDG_CONFIG_HOME/glab-cli/config.yml` (default: `~/.config/glab-cli/config.yml`) +3. `$XDG_CONFIG_DIRS/glab-cli/config.yml` (default: `/etc/xdg/glab-cli/config.yml`) + +The first configuration file found is used. This allows system administrators to provide +site-wide defaults while allowing individual users to override them. + ### Configure `glab` to use your GitLab Self-Managed or GitLab Dedicated instance When outside a Git repository, `glab` uses `gitlab.com` by default. For `glab` to default diff --git a/go.mod b/go.mod index 66187c6c9..975f66e87 100644 --- a/go.mod +++ b/go.mod @@ -12,6 +12,7 @@ require ( github.com/AlecAivazis/survey/v2 v2.3.7 github.com/MakeNowJust/heredoc/v2 v2.0.1 github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d + github.com/adrg/xdg v0.5.3 github.com/avast/retry-go/v4 v4.7.0 github.com/briandowns/spinner v1.23.2 github.com/charmbracelet/glamour v0.10.0 diff --git a/go.sum b/go.sum index 8f17ed899..cddaa73ec 100644 --- a/go.sum +++ b/go.sum @@ -14,6 +14,8 @@ github.com/PuerkitoBio/goquery v1.10.0 h1:6fiXdLuUvYs2OJSvNRqlNPoBm6YABE226xrbav github.com/PuerkitoBio/goquery v1.10.0/go.mod h1:TjZZl68Q3eGHNBA8CWaxAN7rOU1EbDz3CWuolcO5Yu4= github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d h1:licZJFw2RwpHMqeKTCYkitsPqHNxTmd4SNR5r94FGM8= github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d/go.mod h1:asat636LX7Bqt5lYEZ27JNDcqxfjdBQuJ/MM4CN/Lzo= +github.com/adrg/xdg v0.5.3 h1:xRnxJXne7+oWDatRhR1JLnvuccuIeCoBu2rtuLqQB78= +github.com/adrg/xdg v0.5.3/go.mod h1:nlTsY+NNiCBGCK2tpm09vRqfVzrc2fLmXGpBLF0zlTQ= github.com/alecthomas/assert/v2 v2.7.0 h1:QtqSACNS3tF7oasA8CU6A6sXZSBDqnm7RfpLl9bZqbE= github.com/alecthomas/assert/v2 v2.7.0/go.mod h1:Bze95FyfUr7x34QZrjL+XP+0qgp/zg8yS+TtBj1WA3k= github.com/alecthomas/chroma/v2 v2.14.0 h1:R3+wzpnUArGcQz7fCETQBzO5n9IMNi13iIs46aU4V9E= diff --git a/internal/config/config_file.go b/internal/config/config_file.go index 342c79370..4a48c0f5d 100644 --- a/internal/config/config_file.go +++ b/internal/config/config_file.go @@ -8,7 +8,7 @@ import ( "path/filepath" "syscall" - "github.com/mitchellh/go-homedir" + "github.com/adrg/xdg" "gopkg.in/yaml.v3" ) @@ -17,27 +17,62 @@ var ( configError error ) -// ConfigDir returns the config directory +// ConfigDir returns the config directory for writing configuration. +// It respects GLAB_CONFIG_DIR as the highest priority override, +// otherwise uses XDG_CONFIG_HOME (defaulting to ~/.config). func ConfigDir() string { glabDir := os.Getenv("GLAB_CONFIG_DIR") if glabDir != "" { return glabDir } - usrConfigHome := os.Getenv("XDG_CONFIG_HOME") - if usrConfigHome == "" { - usrConfigHome = os.Getenv("HOME") - if usrConfigHome == "" { - usrConfigHome, _ = homedir.Expand("~/.config") - } else { - usrConfigHome = filepath.Join(usrConfigHome, ".config") - } - } - return filepath.Join(usrConfigHome, "glab-cli") + return filepath.Join(xdg.ConfigHome, "glab-cli") } -// ConfigFile returns the config file path +// ConfigFile returns the config file path for writing configuration. +// It respects GLAB_CONFIG_DIR as the highest priority override, +// otherwise uses XDG conventions for the config file location. func ConfigFile() string { - return path.Join(ConfigDir(), "config.yml") + glabDir := os.Getenv("GLAB_CONFIG_DIR") + if glabDir != "" { + return filepath.Join(glabDir, "config.yml") + } + // Use xdg.ConfigFile to get a writable config location + // This will create parent directories if needed + configPath, err := xdg.ConfigFile("glab-cli/config.yml") + if err != nil { + // Fall back to manual path construction if directory creation fails + // This can happen due to permission issues or read-only filesystems + return filepath.Join(xdg.ConfigHome, "glab-cli", "config.yml") + } + return configPath +} + +// SearchConfigFile searches for an existing config file across all XDG config paths. +// It respects GLAB_CONFIG_DIR as the highest priority override. +// Search order: +// 1. $GLAB_CONFIG_DIR/config.yml (if GLAB_CONFIG_DIR is set) +// 2. $XDG_CONFIG_HOME/glab-cli/config.yml (default: ~/.config/glab-cli/config.yml) +// 3. $XDG_CONFIG_DIRS/glab-cli/config.yml (default: /etc/xdg/glab-cli/config.yml) +// +// Returns the path to the first config file found, or an error if none exist. +func SearchConfigFile() (string, error) { + // HIGHEST PRIORITY: GLAB_CONFIG_DIR completely bypasses XDG + if glabDir := os.Getenv("GLAB_CONFIG_DIR"); glabDir != "" { + configPath := filepath.Join(glabDir, "config.yml") + if _, err := os.Stat(configPath); err == nil { + return configPath, nil + } + // If GLAB_CONFIG_DIR is set but file doesn't exist, + // still return this path (don't fall through to XDG) + return configPath, os.ErrNotExist + } + + // XDG search: user config → system configs + configPath, err := xdg.SearchConfigFile("glab-cli/config.yml") + if err != nil { + return "", err + } + return configPath, nil } // Init initialises and returns the cached configuration @@ -57,7 +92,13 @@ func Init() (Config, error) { } func ParseDefaultConfig() (Config, error) { - return ParseConfig(ConfigFile()) + // Try to find existing config first (searches all XDG paths) + configPath, err := SearchConfigFile() + if err != nil { + // No config found, use default writable location + configPath = ConfigFile() + } + return ParseConfig(configPath) } var ReadConfigFile = func(filename string) ([]byte, error) { diff --git a/internal/config/config_file_test.go b/internal/config/config_file_test.go index 405b2a300..02f8ac938 100644 --- a/internal/config/config_file_test.go +++ b/internal/config/config_file_test.go @@ -7,6 +7,7 @@ import ( "reflect" "testing" + "github.com/adrg/xdg" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/cli/test" @@ -293,3 +294,184 @@ host: https://gitlab.mycompany.org eq(t, err, nil) eq(t, val, "https://gitlab.mycompany.org") } + +func Test_SearchConfigFile_UserConfig(t *testing.T) { + test.ClearEnvironmentVariables(t) + + // Create a temporary user config directory + userConfigDir := t.TempDir() + userConfigFile := filepath.Join(userConfigDir, "glab-cli", "config.yml") + + // Create config file + err := os.MkdirAll(filepath.Dir(userConfigFile), 0o750) + require.NoError(t, err) + err = os.WriteFile(userConfigFile, []byte("test: user"), 0o600) + require.NoError(t, err) + + // Set XDG_CONFIG_HOME to temp directory + t.Setenv("XDG_CONFIG_HOME", userConfigDir) + xdg.Reload() // Reload XDG paths after env change + + // SearchConfigFile should find the user config + foundPath, err := SearchConfigFile() + require.NoError(t, err) + assert.Equal(t, userConfigFile, foundPath) +} + +func Test_SearchConfigFile_SystemConfig(t *testing.T) { + test.ClearEnvironmentVariables(t) + + // Create temporary directories for system and user configs + userConfigDir := t.TempDir() + systemConfigDir := t.TempDir() + systemConfigFile := filepath.Join(systemConfigDir, "glab-cli", "config.yml") + + // Create ONLY system config (no user config) + err := os.MkdirAll(filepath.Dir(systemConfigFile), 0o750) + require.NoError(t, err) + err = os.WriteFile(systemConfigFile, []byte("test: system"), 0o600) + require.NoError(t, err) + + // Set XDG environment variables + t.Setenv("XDG_CONFIG_HOME", userConfigDir) + t.Setenv("XDG_CONFIG_DIRS", systemConfigDir) + xdg.Reload() // Reload XDG paths after env change + + // SearchConfigFile should find the system config + foundPath, err := SearchConfigFile() + require.NoError(t, err) + assert.Equal(t, systemConfigFile, foundPath) +} + +func Test_SearchConfigFile_Precedence(t *testing.T) { + test.ClearEnvironmentVariables(t) + + // Create temporary directories + userConfigDir := t.TempDir() + systemConfigDir := t.TempDir() + userConfigFile := filepath.Join(userConfigDir, "glab-cli", "config.yml") + systemConfigFile := filepath.Join(systemConfigDir, "glab-cli", "config.yml") + + // Create both user and system configs + err := os.MkdirAll(filepath.Dir(userConfigFile), 0o750) + require.NoError(t, err) + err = os.WriteFile(userConfigFile, []byte("test: user"), 0o600) + require.NoError(t, err) + + err = os.MkdirAll(filepath.Dir(systemConfigFile), 0o750) + require.NoError(t, err) + err = os.WriteFile(systemConfigFile, []byte("test: system"), 0o600) + require.NoError(t, err) + + // Set XDG environment variables + t.Setenv("XDG_CONFIG_HOME", userConfigDir) + t.Setenv("XDG_CONFIG_DIRS", systemConfigDir) + xdg.Reload() // Reload XDG paths after env change + + // SearchConfigFile should prefer user config over system config + foundPath, err := SearchConfigFile() + require.NoError(t, err) + assert.Equal(t, userConfigFile, foundPath) +} + +func Test_SearchConfigFile_NotFound(t *testing.T) { + test.ClearEnvironmentVariables(t) + + // Use empty temp directories (no config files) + userConfigDir := t.TempDir() + systemConfigDir := t.TempDir() + + t.Setenv("XDG_CONFIG_HOME", userConfigDir) + t.Setenv("XDG_CONFIG_DIRS", systemConfigDir) + xdg.Reload() // Reload XDG paths after env change + + // SearchConfigFile should return an error + _, err := SearchConfigFile() + require.Error(t, err) +} + +func Test_SearchConfigFile_GLABConfigDirOverride(t *testing.T) { + test.ClearEnvironmentVariables(t) + + // Create temp directories + glabConfigDir := t.TempDir() + userConfigDir := t.TempDir() + glabConfigFile := filepath.Join(glabConfigDir, "config.yml") + userConfigFile := filepath.Join(userConfigDir, "glab-cli", "config.yml") + + // Create both GLAB_CONFIG_DIR and user configs + err := os.WriteFile(glabConfigFile, []byte("test: glab"), 0o600) + require.NoError(t, err) + + err = os.MkdirAll(filepath.Dir(userConfigFile), 0o750) + require.NoError(t, err) + err = os.WriteFile(userConfigFile, []byte("test: user"), 0o600) + require.NoError(t, err) + + // Set both environment variables + t.Setenv("GLAB_CONFIG_DIR", glabConfigDir) + t.Setenv("XDG_CONFIG_HOME", userConfigDir) + + // GLAB_CONFIG_DIR should take precedence + foundPath, err := SearchConfigFile() + require.NoError(t, err) + assert.Equal(t, glabConfigFile, foundPath) +} + +func Test_SearchConfigFile_GLABConfigDirNotFound(t *testing.T) { + test.ClearEnvironmentVariables(t) + + // Use empty GLAB_CONFIG_DIR + glabConfigDir := t.TempDir() + t.Setenv("GLAB_CONFIG_DIR", glabConfigDir) + + // Should return os.ErrNotExist, not search XDG paths + _, err := SearchConfigFile() + require.ErrorIs(t, err, os.ErrNotExist) +} + +func Test_ConfigFile_XDGCompliance(t *testing.T) { + test.ClearEnvironmentVariables(t) + + userConfigDir := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", userConfigDir) + xdg.Reload() // Reload XDG paths after env change + + configFile := ConfigFile() + expectedPath := filepath.Join(userConfigDir, "glab-cli", "config.yml") + + assert.Equal(t, expectedPath, configFile) +} + +func Test_ConfigDir_XDGCompliance(t *testing.T) { + test.ClearEnvironmentVariables(t) + + userConfigDir := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", userConfigDir) + xdg.Reload() // Reload XDG paths after env change + + configDir := ConfigDir() + expectedPath := filepath.Join(userConfigDir, "glab-cli") + + assert.Equal(t, expectedPath, configDir) +} + +func Test_ConfigFile_ErrorHandling(t *testing.T) { + test.ClearEnvironmentVariables(t) + + // Set XDG_CONFIG_HOME to a path that might cause issues + // In this test, we just verify ConfigFile() always returns a valid path + userConfigDir := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", userConfigDir) + xdg.Reload() // Reload XDG paths after env change + + configFile := ConfigFile() + + // Should always return a path, even if directory creation might fail later + assert.NotEmpty(t, configFile) + assert.Contains(t, configFile, "glab-cli") + assert.Contains(t, configFile, "config.yml") + + // Path should be under the user config directory + assert.True(t, filepath.IsAbs(configFile), "ConfigFile should return an absolute path") +} -- GitLab