Skip to content

Commit ba2ff2e

Browse files
committed
Fix auth switch writing to a stale config path
`auth switch` writes the updated config to `viper.GetString("config")`, but `writeConfig` persists `viper.AllSettings()` — which includes the bound `config` flag — so a top-level `config:` key is saved into the file. On the next run viper reads it back, and because a config-file value outranks a flag's default, `GetString("config")` returns that stored path. When a config is copied between machines the path is stale, so `auth switch` silently writes there and prints success while the loaded config is left unchanged. Write to `viper.ConfigFileUsed()` — the path resolved at init, before the config file is read — so the write target can't be hijacked by the persisted key. Add a regression test covering a config that carries a stale `config:`. Fixes #1882
1 parent 92915f6 commit ba2ff2e

2 files changed

Lines changed: 53 additions & 1 deletion

File tree

commands/auth.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,18 @@ func writeConfig() error {
355355
// When using the default config file path the default config home directory will be created; Otherwise
356356
// the custom config home directory must exist and be writable to the user issuing the auth command.
357357
func defaultConfigFileWriter() (io.WriteCloser, error) {
358-
cfgFile := viper.GetString("config")
358+
// Write to the config file viper actually loaded, not the value of the
359+
// "config" key. writeConfig persists viper.AllSettings(), which includes the
360+
// bound "config" flag, so a top-level "config:" entry ends up in the file. On
361+
// a later run viper reads it back, and because a config-file value outranks a
362+
// flag's default, viper.GetString("config") returns that stored path. When the
363+
// config was copied from another machine the path is stale, and auth switch
364+
// would silently write there instead of the live config. ConfigFileUsed() is
365+
// the path set at init, before the file is read, so it is not hijacked.
366+
cfgFile := viper.ConfigFileUsed()
367+
if cfgFile == "" {
368+
cfgFile = viper.GetString("config")
369+
}
359370

360371
defaultCfgFile := filepath.Join(defaultConfigHome(), defaultConfigName)
361372
if cfgFile == defaultCfgFile {

commands/auth_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"bufio"
1818
"bytes"
1919
"io"
20+
"os"
2021
"path/filepath"
2122
"testing"
2223

@@ -26,6 +27,7 @@ import (
2627
"github.com/digitalocean/doctl/do"
2728
"github.com/spf13/viper"
2829
"github.com/stretchr/testify/assert"
30+
"github.com/stretchr/testify/require"
2931
"go.uber.org/mock/gomock"
3032
yaml "gopkg.in/yaml.v2"
3133
)
@@ -162,6 +164,45 @@ func TestAuthForcesLowercase(t *testing.T) {
162164
})
163165
}
164166

167+
func TestDefaultConfigFileWriterUsesLoadedConfig(t *testing.T) {
168+
// Regression: a config carrying a stale top-level "config:" path (e.g. copied
169+
// from another machine) must not hijack where auth switch writes. The write
170+
// must land in the config file viper actually loaded.
171+
origContext := viper.Get("context")
172+
origConfig := viper.Get("config")
173+
origFile := viper.ConfigFileUsed()
174+
defer func() {
175+
viper.Set("context", origContext)
176+
viper.Set("config", origConfig)
177+
viper.SetConfigFile(origFile)
178+
}()
179+
180+
dir := t.TempDir()
181+
realCfg := filepath.Join(dir, "config.yaml")
182+
staleCfg := filepath.Join(dir, "copied-from-another-host.yaml")
183+
184+
require.NoError(t, os.WriteFile(realCfg,
185+
[]byte("context: default\nconfig: "+staleCfg+"\n"), 0600))
186+
187+
// Mimic initConfig: point viper at the real file and read it in.
188+
viper.SetConfigFile(realCfg)
189+
require.NoError(t, viper.ReadInConfig())
190+
191+
// The loaded file now carries `config: <staleCfg>`. The pre-fix code used
192+
// viper.GetString("config") as the write target and would land on staleCfg;
193+
// the assertions below prove the writer targets the loaded file instead.
194+
w, err := defaultConfigFileWriter()
195+
require.NoError(t, err)
196+
_, err = io.WriteString(w, "context: newcontext\n")
197+
require.NoError(t, err)
198+
require.NoError(t, w.Close())
199+
200+
assert.NoFileExists(t, staleCfg, "must not write to the stale config path")
201+
got, err := os.ReadFile(realCfg)
202+
require.NoError(t, err)
203+
assert.Contains(t, string(got), "newcontext", "must rewrite the loaded config file")
204+
}
205+
165206
func TestAuthList(t *testing.T) {
166207
buf := &bytes.Buffer{}
167208
config := &CmdConfig{Out: buf}

0 commit comments

Comments
 (0)