From 4752c19f23c268751bcb22fe069f847af7f35cbc Mon Sep 17 00:00:00 2001 From: Tim Walsh Date: Fri, 28 Aug 2026 01:08:14 -0700 Subject: [PATCH] fix(config): refuse plaintext credentials and redact them in config list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `cu config set api_token ` wrote the token to ~/.config/cu/config.yaml in plaintext — and then never used it, since auth reads the OS keyring only. The operation had no upside: it leaked a secret to disk and left `cu auth status` reporting "Not authenticated". This was an asymmetry introduced with the .cu.yml credential guard: project files were filtered, but Set staged every key including credentials. `cu config set` now refuses credential keys and points at `cu auth login`, and config.Set no longer stages them as a backstop — they still apply in-process, so nothing that relies on a runtime override breaks. `cu config list` redacts credential values, since that output is pasted into issues and terminals far more casually than an explicit `config get `, which is left alone as a deliberate act. A token already in a config file is preserved rather than silently deleted; refusing to add more is the fix, and list redacts what is already there. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014aqbmccWm1tqttmBUCR5rv ClickUp: 86dxbeqyt --- internal/cmd/config.go | 16 +++++++++++++ internal/config/config.go | 32 +++++++++++++++++++++++--- internal/config/config_test.go | 41 ++++++++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 3 deletions(-) diff --git a/internal/cmd/config.go b/internal/cmd/config.go index 618a5e8..4f467b4 100644 --- a/internal/cmd/config.go +++ b/internal/cmd/config.go @@ -31,6 +31,14 @@ var configListCmd = &cobra.Command{ sort.Strings(keys) for _, key := range keys { + // A token here predates the refusal in `config set`, or was added + // by hand. Never print it: `config list` output gets pasted into + // issues and terminals far more casually than an explicit + // `config get ` does. + if config.IsCredentialKey(key) { + fmt.Printf("%s=%s\n", key, config.RedactedValue) + continue + } fmt.Printf("%s=%v\n", key, settings[key]) } }, @@ -61,6 +69,14 @@ var configSetCmd = &cobra.Command{ key := args[0] value := args[1] + // Credentials do not belong in a plaintext config file, and cu would + // not read one back if they were — authentication uses the keyring. + if config.IsCredentialKey(key) { + fmt.Fprintf(os.Stderr, "Refusing to write %q to the config file — it would be stored in plaintext and never used.\n", key) + fmt.Fprintln(os.Stderr, "Authenticate with 'cu auth login' instead; the token is kept in your system keyring.") + os.Exit(1) + } + // Handle boolean values if strings.ToLower(value) == "true" || strings.ToLower(value) == "false" { config.Set(key, strings.ToLower(value) == "true") diff --git a/internal/config/config.go b/internal/config/config.go index ff5f0ea..38e6459 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -45,11 +45,28 @@ var ( staged = map[string]interface{}{} ) -// credentialKeys are never accepted from a project .cu.yml. That file is -// committed and reviewed like code, so honouring a token there would let any -// repository you clone substitute the credential used for API calls. +// credentialKeys never reach a config file. They are refused from a project +// .cu.yml — that file is committed and reviewed like code, so honouring a token +// there would let any repository you clone substitute the credential used for +// API calls — and they are never staged for the global config either, since +// credentials belong in the OS keyring, not a plaintext YAML file. var credentialKeys = []string{"api_token"} +// IsCredentialKey reports whether a config key holds a credential. +func IsCredentialKey(key string) bool { + for _, k := range credentialKeys { + if strings.EqualFold(key, k) { + return true + } + } + return false +} + +// RedactedValue is substituted for credential values in any bulk output. It +// deliberately does not claim where the value lives: a key found here is a +// plaintext leftover, not the keyring entry cu actually authenticates with. +const RedactedValue = "" + // globalPath returns the global config file to write. An explicit --config // always wins; otherwise a discovered file is used only while it still lives // under the configured directory, since DefaultConfigDir is a variable that @@ -172,8 +189,17 @@ func Get(key string) interface{} { // Set sets a configuration value for this process and stages it for the global // config file, so a following Save persists it there. +// +// Credential keys are applied in-process but never staged: writing them to +// ~/.config/cu/config.yaml would put a secret on disk in plaintext, and nothing +// reads it back — authentication goes through the OS keyring. Callers that take +// a key from the user should refuse it outright via IsCredentialKey rather than +// relying on this backstop, so the user gets told instead of silently ignored. func Set(key string, value interface{}) { viper.Set(key, value) + if IsCredentialKey(key) { + return + } staged[key] = value } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index d8c1235..34bcb42 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -613,3 +613,44 @@ func TestWorkingDirectoryIsNotAGlobalConfigSource(t *testing.T) { assert.NotEqual(t, filepath.Join(repo, ConfigFileName+"."+ConfigType), viper.ConfigFileUsed(), "the working directory must not be searched for the global config") } + +func TestCredentialKeysAreNeverStaged(t *testing.T) { + t.Run("IsCredentialKey", func(t *testing.T) { + assert.True(t, IsCredentialKey("api_token")) + assert.True(t, IsCredentialKey("API_TOKEN"), "matching is case-insensitive") + assert.False(t, IsCredentialKey("default_list")) + }) + + t.Run("Set applies in-process but does not persist", func(t *testing.T) { + cfgDir, _ := newLayeredFixture(t, "default_space: global-space\n", "default_list: from-project\n") + require.NoError(t, Init("")) + + Set("api_token", "sk-should-not-be-written") + Set("default_list", "persisted") + require.NoError(t, Save()) + + // Available to the running process... + assert.Equal(t, "sk-should-not-be-written", GetString("api_token")) + + // ...but never written to disk. + written, err := os.ReadFile(filepath.Join(cfgDir, ConfigFileName+"."+ConfigType)) + require.NoError(t, err) + assert.NotContains(t, string(written), "sk-should-not-be-written", + "a credential must never reach the config file") + assert.Contains(t, string(written), "persisted", "ordinary keys are still saved") + }) + + t.Run("a pre-existing plaintext token is preserved, not silently dropped", func(t *testing.T) { + // Deleting a user's data would be a surprise; refusing to add more is + // the fix. `config list` redacts whatever is already there. + cfgDir, _ := newLayeredFixture(t, "api_token: legacy-token\n", "default_list: from-project\n") + require.NoError(t, Init("")) + + Set("default_list", "x") + require.NoError(t, Save()) + + written, err := os.ReadFile(filepath.Join(cfgDir, ConfigFileName+"."+ConfigType)) + require.NoError(t, err) + assert.Contains(t, string(written), "legacy-token") + }) +}