From e46f72a9b9c8620df1b3f63d865a60fa8e5d15f0 Mon Sep 17 00:00:00 2001 From: Ricardo Chaves Date: Tue, 1 Sep 2026 10:38:30 +0100 Subject: [PATCH 1/2] fix: redact access token in config --list output The access token stored in the config file was rendered verbatim by `ldcli config --list` in every output mode, including the JSON default used whenever stdout is not a terminal. That put the secret into terminal scrollback, shell history captures, and piped CI logs. Add Config.Redacted() and marshal that for output instead. Because the plaintext, --output json and non-TTY JSON paths all derive from the same marshal, one substitution covers all three. The value written to the config file is unchanged, and an unset token stays elided by omitempty rather than being reported as present but hidden. --- cmd/config/config.go | 2 +- internal/config/config.go | 15 +++++++ internal/config/config_test.go | 75 ++++++++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 1 deletion(-) diff --git a/cmd/config/config.go b/cmd/config/config.go index 186faf74e..ad186feb2 100644 --- a/cmd/config/config.go +++ b/cmd/config/config.go @@ -103,7 +103,7 @@ func run(service config.Service) func(*cobra.Command, []string) error { return newErr(err.Error()) } - configJSON, err := json.Marshal(conf) + configJSON, err := json.Marshal(conf.Redacted()) if err != nil { return newErr(err.Error()) } diff --git a/internal/config/config.go b/internal/config/config.go index b6f720042..aa9268539 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -16,6 +16,10 @@ import ( const Filename = ".ldcli-config.yml" +// RedactedValue is rendered in place of a sensitive configuration value. It is only ever +// substituted into command output; the value stored in the config file is left untouched. +const RedactedValue = "[REDACTED]" + type ReadFile func(name string) ([]byte, error) // Config represents the data stored in the config file. @@ -47,6 +51,17 @@ func New(filename string, readFile ReadFile) (Config, error) { return c, nil } +// Redacted returns a copy of the Config with sensitive values replaced by RedactedValue, for use +// anywhere a Config is rendered to output. An unset sensitive value is left empty so that it stays +// elided by omitempty rather than being reported as a value that is present but hidden. +func (c Config) Redacted() Config { + if c.AccessToken != "" { + c.AccessToken = RedactedValue + } + + return c +} + // Update validates the updating fields and sets them on the Config. It returns the updated fields // in addition to the Config. func (c Config) Update(kvs []string) (Config, []string, error) { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 46b9ef317..92e1730c0 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1,6 +1,7 @@ package config_test import ( + "encoding/json" "errors" "net/http" "testing" @@ -10,6 +11,7 @@ import ( "gopkg.in/yaml.v3" "github.com/launchdarkly/ldcli/internal/config" + "github.com/launchdarkly/ldcli/internal/output" "github.com/launchdarkly/ldcli/internal/resources" ) @@ -149,3 +151,76 @@ func TestRemove(t *testing.T) { assert.EqualError(t, err, "invalid is not a valid configuration option") }) } + +func TestRedacted(t *testing.T) { + t.Run("replaces a set access token", func(t *testing.T) { + c := config.Config{AccessToken: "test-access-token"} + + redacted := c.Redacted() + + assert.Equal(t, config.RedactedValue, redacted.AccessToken) + }) + + t.Run("leaves an unset access token empty so omitempty still elides it", func(t *testing.T) { + c := config.Config{Project: "test-project"} + + redacted := c.Redacted() + + assert.Equal(t, "", redacted.AccessToken) + + configJSON, err := json.Marshal(redacted) + require.NoError(t, err) + assert.NotContains(t, string(configJSON), "access-token") + }) + + t.Run("leaves non-sensitive values alone", func(t *testing.T) { + optOut := true + c := config.Config{ + AccessToken: "test-access-token", + AnalyticsOptOut: &optOut, + BaseURI: "http://test.com", + DevStreamURI: "http://stream.test.com", + Environment: "test-environment", + Flag: "test-flag", + Output: "json", + Project: "test-project", + } + + redacted := c.Redacted() + + expected := c + expected.AccessToken = config.RedactedValue + assert.Equal(t, expected, redacted) + }) + + t.Run("does not mutate the receiver", func(t *testing.T) { + c := config.Config{AccessToken: "test-access-token"} + + _ = c.Redacted() + + assert.Equal(t, "test-access-token", c.AccessToken) + }) +} + +// TestRedactedOutput covers the rendering paths that `config --list` feeds the redacted Config +// into, so that neither the plaintext nor the JSON representation can reveal the token. +func TestRedactedOutput(t *testing.T) { + c := config.Config{ + AccessToken: "test-access-token", + Project: "test-project", + } + + configJSON, err := json.Marshal(c.Redacted()) + require.NoError(t, err) + + for _, outputKind := range []string{"json", "plaintext"} { + t.Run(outputKind, func(t *testing.T) { + out, err := output.CmdOutputSingular(outputKind, configJSON, output.ConfigPlaintextOutputFn) + + require.NoError(t, err) + assert.NotContains(t, out, "test-access-token") + assert.Contains(t, out, config.RedactedValue) + assert.Contains(t, out, "test-project") + }) + } +} From b737ff21252d5aafdd002d91b15cb3b96e99d996 Mon Sep 17 00:00:00 2001 From: Ricardo Chaves Date: Tue, 1 Sep 2026 10:39:20 +0100 Subject: [PATCH 2/2] fix: do not echo non-key arguments in config validation errors `config --set` and `config --unset` reject an unknown key by echoing it back. The argument in that position is not always a key: transposing `--set ` puts the value there, so `--set access-token` printed the token inside the error message. Echo the argument only when it has the shape of a configuration key. Typos, which are the reason the argument is echoed at all, still appear verbatim. --- internal/config/config.go | 25 +++++++++++++++++++++++-- internal/config/config_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index aa9268539..72b2fd803 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "path/filepath" + "regexp" "strconv" "github.com/mitchellh/go-homedir" @@ -62,6 +63,26 @@ func (c Config) Redacted() Config { return c } +// configKeyPattern describes the shape of every supported configuration key: lowercase words +// joined by single hyphens. +var configKeyPattern = regexp.MustCompile(`^[a-z0-9]+(-[a-z0-9]+)*$`) + +// maxConfigKeyLength bounds what is treated as key-shaped. The longest supported key is well +// under this, while an access token is well over it. +const maxConfigKeyLength = 32 + +// safeKeyForError returns key when it has the shape of a configuration key, and RedactedValue +// otherwise. An unrecognized key is echoed back to help the user spot a typo, but the argument in +// that position is not always a key: transposing `--set ` puts the value there, and +// for access-token that value is a secret. Anything that is not key-shaped is therefore elided. +func safeKeyForError(key string) string { + if len(key) <= maxConfigKeyLength && configKeyPattern.MatchString(key) { + return key + } + + return RedactedValue +} + // Update validates the updating fields and sets them on the Config. It returns the updated fields // in addition to the Config. func (c Config) Update(kvs []string) (Config, []string, error) { @@ -74,7 +95,7 @@ func (c Config) Update(kvs []string) (Config, []string, error) { // TODO: move this list to this package? _, ok := cliflags.AllFlagsHelp()[kvs[i]] if !ok { - return Config{}, updatedFields, errors.NewError(fmt.Sprintf("%s is not a valid configuration option", kvs[i])) + return Config{}, updatedFields, errors.NewError(fmt.Sprintf("%s is not a valid configuration option", safeKeyForError(kvs[i]))) } } @@ -122,7 +143,7 @@ func (c Config) Update(kvs []string) (Config, []string, error) { func (c Config) Remove(key string) (Config, error) { _, ok := cliflags.AllFlagsHelp()[key] if !ok { - return Config{}, errors.NewError(fmt.Sprintf("%s is not a valid configuration option", key)) + return Config{}, errors.NewError(fmt.Sprintf("%s is not a valid configuration option", safeKeyForError(key))) } return c, nil diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 92e1730c0..3078d1cce 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -224,3 +224,30 @@ func TestRedactedOutput(t *testing.T) { }) } } + +// TestErrorDoesNotEchoNonKeyArguments covers the `--set`/`--unset` validation errors, which echo +// back the argument they rejected. A transposed `--set access-token` puts a secret in the +// position a key was expected in, so only key-shaped arguments are echoed. +func TestErrorDoesNotEchoNonKeyArguments(t *testing.T) { + const token = "api-2c2f9f1e-0b1a-4a3a-9d3f-000000000000" + + t.Run("a key-shaped typo is still echoed", func(t *testing.T) { + _, _, err := config.Config{}.Update([]string{"projct", "test-project"}) + + assert.EqualError(t, err, "projct is not a valid configuration option") + }) + + t.Run("a transposed --set does not echo the value", func(t *testing.T) { + _, _, err := config.Config{}.Update([]string{token, "access-token"}) + + assert.EqualError(t, err, config.RedactedValue+" is not a valid configuration option") + assert.NotContains(t, err.Error(), token) + }) + + t.Run("--unset does not echo a non-key argument", func(t *testing.T) { + _, err := config.Config{}.Remove(token) + + assert.EqualError(t, err, config.RedactedValue+" is not a valid configuration option") + assert.NotContains(t, err.Error(), token) + }) +}