fix(config)!: stop loading the config file from the working directory

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)

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")
+}

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