From 883e90bb624ffe35c1e5474f3f00a8bbf670bc12 Mon Sep 17 00:00:00 2001 From: Peter Beckham Date: Tue, 8 Sep 2026 16:08:06 +0100 Subject: [PATCH] fix!: stop loading the Kosli config file from the working directory The --config-file default fell back to the bare name "kosli" whenever $HOME/.kosli.yml was absent, which viper resolves against the current working directory. A checkout could therefore ship kosli.yml and set any flag for a command run with a real KOSLI_API_TOKEN: host or http-proxy sends the bearer token to a host of the repository's choosing, and the snapshot k8s kubeconfig flag runs a kubeconfig exec credential plugin, which --dry-run does not prevent. The default is now always the home config file, whether or not it exists. A config file in the working directory is loaded only when the user names it with --config-file or KOSLI_CONFIG_FILE. When no home directory can be resolved there is no default config file at all, rather than a bare name that reopens the working-directory search, and `kosli config` says so instead of writing a config file into the working directory. A warning names an ignored working-directory config so the change is not silent. It fires only for a file that sets org, api-token or host: a kosli.yml in a repository root is far more often a flow template, which was never loaded as CLI config. Refs kosli-dev/server#6778, kosli-dev/server#6779 BREAKING CHANGE: a kosli.{yaml,yml,json,toml} file in the current working directory is no longer loaded automatically. Pass --config-file, set KOSLI_CONFIG_FILE, or move the settings to $HOME/.kosli.yml with `kosli config`. Co-Authored-By: Claude Opus 5 (1M context) --- cmd/kosli/config.go | 6 + cmd/kosli/configWorkingDir_test.go | 174 +++++++++++++++++++++++++++++ cmd/kosli/root.go | 105 ++++++++++++----- 3 files changed, 258 insertions(+), 27 deletions(-) create mode 100644 cmd/kosli/configWorkingDir_test.go diff --git a/cmd/kosli/config.go b/cmd/kosli/config.go index 8d64250fb..38b0b6068 100644 --- a/cmd/kosli/config.go +++ b/cmd/kosli/config.go @@ -79,6 +79,12 @@ func newConfigCmd(out io.Writer) *cobra.Command { func (o *configOptions) run() error { path := defaultConfigFilePathFunc() + // An empty path means no home directory could be resolved. Continuing would + // write the config into the current working directory, which is never where + // the default config file belongs. + if path == "" { + return fmt.Errorf("setting default config failed. Could not determine your home directory. Set HOME, or use --config-file on each command instead") + } home := filepath.Dir(path) configFileName := filepath.Base(path) permissions := os.FileMode(0600) diff --git a/cmd/kosli/configWorkingDir_test.go b/cmd/kosli/configWorkingDir_test.go new file mode 100644 index 000000000..b224fd8b6 --- /dev/null +++ b/cmd/kosli/configWorkingDir_test.go @@ -0,0 +1,174 @@ +package main + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/suite" +) + +// WorkingDirConfigTestSuite covers the config file the CLI must NOT load: one +// sitting in the current working directory that the user never named. Loading +// it lets the contents of a checkout set host, http-proxy or kubeconfig for a +// command run with a real API token (kosli-dev/server#6778, #6779). +type WorkingDirConfigTestSuite struct { + suite.Suite +} + +func (suite *WorkingDirConfigTestSuite) TearDownTest() { + defaultConfigFilePathFunc = (&RealConfigGetter{}).defaultConfigFilePath + global = new(GlobalOpts) +} + +// stubHomeConfig points the default config path at a file that does not exist, +// which is the state that used to trigger the working-directory fallback. +func (suite *WorkingDirConfigTestSuite) stubHomeConfig() string { + path := filepath.Join(suite.T().TempDir(), defaultConfigFilename) + mockConfigGetter := new(MockConfigGetter) + mockConfigGetter.Mock.On("defaultConfigFilePath").Return(path) + defaultConfigFilePathFunc = mockConfigGetter.defaultConfigFilePath + return path +} + +// chdirWithConfig writes content to a named config file in a temp directory and +// makes that directory the working directory for the test. +func (suite *WorkingDirConfigTestSuite) chdirWithConfig(name, content string) { + dir := suite.T().TempDir() + suite.Require().NoError(os.WriteFile(filepath.Join(dir, name), []byte(content), 0600)) + suite.T().Chdir(dir) +} + +func (suite *WorkingDirConfigTestSuite) TestDefaultIsHomePathWhenHomeConfigIsAbsent() { + path := suite.stubHomeConfig() + + _, _, _, _, err := executeCommandC("version") + + suite.Require().NoError(err) + suite.Equal(path, global.ConfigFile, + "the default config file must be the home path, not a bare name that resolves to the working directory") +} + +func (suite *WorkingDirConfigTestSuite) TestWorkingDirConfigIsNotLoaded() { + for _, name := range []string{"kosli.yml", "kosli.yaml", "kosli.json", "kosli.toml"} { + suite.Run(name, func() { + defer func() { global = new(GlobalOpts) }() + suite.stubHomeConfig() + content := "host: https://attacker.example\n" + switch filepath.Ext(name) { + case ".json": + content = `{"host": "https://attacker.example"}` + case ".toml": + content = `host = "https://attacker.example"` + } + suite.chdirWithConfig(name, content) + + _, _, _, _, err := executeCommandC("version") + + suite.Require().NoError(err) + suite.Equal(defaultHost, global.Host, + "a config file in the working directory must not set the host") + }) + } +} + +func (suite *WorkingDirConfigTestSuite) TestExplicitConfigFileFlagStillLoadsWorkingDirConfig() { + suite.stubHomeConfig() + suite.chdirWithConfig("kosli.yml", "host: https://named.example\n") + + _, _, _, stderr, err := executeCommandC("version --config-file kosli.yml") + + suite.Require().NoError(err) + suite.Equal("https://named.example", global.Host, + "a config file the user names must still be loaded") + suite.NotContains(stderr, "no longer loaded automatically", + "naming the file is the supported way to load it, so there is nothing to warn about") +} + +func (suite *WorkingDirConfigTestSuite) TestConfigFileEnvVarStillLoadsWorkingDirConfig() { + suite.stubHomeConfig() + suite.chdirWithConfig("kosli.yml", "host: https://named.example\n") + suite.T().Setenv("KOSLI_CONFIG_FILE", "kosli.yml") + + _, _, _, stderr, err := executeCommandC("version") + + suite.Require().NoError(err) + suite.Equal("https://named.example", global.Host, + "KOSLI_CONFIG_FILE must still be able to name a working-directory file") + suite.NotContains(stderr, "no longer loaded automatically") +} + +// TestEmptyDefaultConfigPathLoadsNothing covers the case where no home +// directory can be resolved, so defaultConfigFilePath returns no path at all. +// The working directory must not become the fallback search location. The empty +// path is injected because homedir.Dir cannot be made to fail portably: on +// macOS it falls back to dscl even with HOME unset. +func (suite *WorkingDirConfigTestSuite) TestEmptyDefaultConfigPathLoadsNothing() { + mockConfigGetter := new(MockConfigGetter) + mockConfigGetter.Mock.On("defaultConfigFilePath").Return("") + defaultConfigFilePathFunc = mockConfigGetter.defaultConfigFilePath + suite.chdirWithConfig("kosli.yml", "host: https://attacker.example\n") + + _, _, _, _, err := executeCommandC("version") + + suite.Require().NoError(err) + suite.Equal(defaultHost, global.Host, + "with no default config path the working directory must not be searched instead") +} + +func (suite *WorkingDirConfigTestSuite) TestWarnsAboutIgnoredWorkingDirConfig() { + cases := []struct { + name string + content string + wantWarn bool + }{ + {name: "host", content: "host: https://attacker.example\n", wantWarn: true}, + {name: "org", content: "org: some-org\n", wantWarn: true}, + {name: "api token", content: "api-token: abc123\n", wantWarn: true}, + {name: "documented uppercase keys", content: "ORG: some-org\nAPI-TOKEN: abc123\n", wantWarn: true}, + // A kosli.yml in a repository root is far more often a flow template, + // which was never loaded as CLI config, so it must stay silent. + {name: "flow template shape", content: "trail:\n artifacts:\n - name: nginx\n", wantWarn: false}, + {name: "unparsable file", content: "\tnot: [valid\n", wantWarn: false}, + } + for _, tc := range cases { + suite.Run(tc.name, func() { + defer func() { global = new(GlobalOpts) }() + suite.stubHomeConfig() + suite.chdirWithConfig("kosli.yml", tc.content) + + _, _, _, stderr, err := executeCommandC("version") + + suite.Require().NoError(err) + if tc.wantWarn { + suite.Contains(stderr, "kosli.yml") + suite.Contains(stderr, "no longer loaded automatically") + suite.Contains(stderr, "--config-file kosli.yml", + "the warning must name the fix, not just the problem") + } else { + suite.NotContains(stderr, "no longer loaded automatically") + } + }) + } +} + +func TestWorkingDirConfigTestSuite(t *testing.T) { + suite.Run(t, new(WorkingDirConfigTestSuite)) +} + +// TestConfigCommandFailsWithoutHomeDirectory pins the other side of an empty +// default config path: `kosli config` must say so rather than silently writing +// a config file into the current working directory. +func (suite *WorkingDirConfigTestSuite) TestConfigCommandFailsWithoutHomeDirectory() { + mockConfigGetter := new(MockConfigGetter) + mockConfigGetter.Mock.On("defaultConfigFilePath").Return("") + defaultConfigFilePathFunc = mockConfigGetter.defaultConfigFilePath + suite.T().Chdir(suite.T().TempDir()) + + _, _, _, _, err := executeCommandC("config --org some-org") + + suite.Require().Error(err) + suite.Contains(err.Error(), "Could not determine your home directory") + _, statErr := os.Stat(defaultConfigFilename) + suite.Require().Error(statErr, "no config file may be written into the working directory") +} diff --git a/cmd/kosli/root.go b/cmd/kosli/root.go index f2d180ce3..2d20a1e4a 100644 --- a/cmd/kosli/root.go +++ b/cmd/kosli/root.go @@ -369,18 +369,57 @@ func (r *RealConfigGetter) defaultConfigFilePath() string { return filepath.Join(home, defaultConfigFilename) } - return "kosli" // for backward compatibility with old default config location + // With no resolvable home directory there is no default config file. A bare + // name here would make viper search the current working directory, which is + // the whole problem getConfigFileFlagDefault exists to avoid. + return "" } // defaultConfigFilePathFunc is a variable holding the implementation of defaultConfigFilePath var defaultConfigFilePathFunc = (&RealConfigGetter{}).defaultConfigFilePath -func getConfigFileFlagDefault() string { - defaultPath := defaultConfigFilePathFunc() - if _, err := os.Stat(defaultPath); err == nil { - return defaultPath +// workingDirConfigNames are the config file names the CLI used to load +// implicitly from the current working directory, before that became +// kosli-dev/server#6778 and #6779. +var workingDirConfigNames = []string{"kosli.yaml", "kosli.yml", "kosli.json", "kosli.toml"} + +// warnAboutIgnoredWorkingDirConfig reports a config file in the current working +// directory that an earlier CLI would have loaded, so that the change does not +// break a pipeline silently. +// +// Only a file that sets a global setting is reported. A kosli.yml in a +// repository root is far more often a flow template, which was never loaded as +// CLI config, and warning about those would be pure noise. +func warnAboutIgnoredWorkingDirConfig() { + for _, name := range workingDirConfigNames { + if _, err := os.Stat(name); err != nil { + continue + } + + v := viper.New() + v.SetConfigFile(name) + if err := v.ReadInConfig(); err != nil { + continue + } + if !v.IsSet("org") && !v.IsSet("api-token") && !v.IsSet("host") { + continue + } + + logger.Warn("config file [%s] in the current directory is no longer loaded automatically. To keep using it, pass --config-file %s or set KOSLI_CONFIG_FILE=%s. To apply its settings to every command, move them to your home config file with 'kosli config'.", name, name, name) + return } - return "kosli" // for backward compatibility with old default config location +} + +// getConfigFileFlagDefault returns the default --config-file value, which is +// always the home config file whether or not that file exists. It used to fall +// back to the bare name "kosli", which viper resolves against the current +// working directory: a repository could then set host, http-proxy or kubeconfig +// for a command run with a real API token, redirecting the bearer token or +// executing a kubeconfig credential plugin (kosli-dev/server#6778, #6779). +// A config file in the working directory is now loaded only when the user names +// it with --config-file or KOSLI_CONFIG_FILE. +func getConfigFileFlagDefault() string { + return defaultConfigFilePathFunc() } func newRootCmd(out, errOut io.Writer, args []string) (*cobra.Command, error) { @@ -530,38 +569,50 @@ func initialize(cmd *cobra.Command, out, errOut io.Writer) error { // we load the config file before we bind env vars to flags, // so we check for the config file env var separately here configFlag := cmd.Flags().Lookup("config-file") + namedByUser := configFlag.Changed if !configFlag.Changed { // A variable set to the empty string reports as present, but it names no // file. Overriding the default with it loads no config file at all, // silently dropping org, api-token and every other configured default. if path, exists := os.LookupEnv("KOSLI_CONFIG_FILE"); exists && path != "" { global.ConfigFile = path + namedByUser = true } } - dir, file := filepath.Split(global.ConfigFile) - file = strings.TrimSuffix(file, filepath.Ext(file)) - // Set the base name of the config file, without the file extension. - v.SetConfigName(file) + if global.ConfigFile != "" { + dir, file := filepath.Split(global.ConfigFile) + file = strings.TrimSuffix(file, filepath.Ext(file)) - // Set as many paths as you like where viper should look for the - // config file. By default, we are looking in the current working directory. - if dir == "" { - dir = "." - } - v.AddConfigPath(dir) - - // Attempt to read the config file, gracefully ignoring errors - // caused by a config file not being found. Return an error - // if we cannot parse the config file. - logger.Debug("processing config file [%s]", global.ConfigFile) - if err := v.ReadInConfig(); err != nil { - // It's okay if there isn't a config file - if _, ok := err.(viper.ConfigFileNotFoundError); !ok { - return fmt.Errorf("failed to parse config file [%s] : %v", global.ConfigFile, err) - } else { - logger.Debug("config file [%s] not found. Skipping.", global.ConfigFile) + // Set the base name of the config file, without the file extension. + v.SetConfigName(file) + + // A relative path is resolved against the current working directory, + // which is what a user asks for by naming one. The default is absolute, + // so it never reaches that case. + if dir == "" { + dir = "." } + v.AddConfigPath(dir) + + // Attempt to read the config file, gracefully ignoring errors + // caused by a config file not being found. Return an error + // if we cannot parse the config file. + logger.Debug("processing config file [%s]", global.ConfigFile) + if err := v.ReadInConfig(); err != nil { + // It's okay if there isn't a config file + if _, ok := err.(viper.ConfigFileNotFoundError); !ok { + return fmt.Errorf("failed to parse config file [%s] : %v", global.ConfigFile, err) + } else { + logger.Debug("config file [%s] not found. Skipping.", global.ConfigFile) + } + } + } else { + logger.Debug("no default config file location could be determined. Skipping.") + } + + if !namedByUser { + warnAboutIgnoredWorkingDirConfig() } // When we bind flags to environment variables expect that the // environment variables are prefixed, e.g. a flag like --namespace