fix: redact access token in config command output - #787
Open
rchaves-nexar wants to merge 2 commits into
Open
Conversation
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.
`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 <key> <value>` puts the value there, so `--set <token> 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.
ffantl-ld
self-requested a review
September 1, 2026 13:59
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Requirements
Related issues
Fixes #786.
Describe the solution you've provided
The stored access token was rendered verbatim by
config --listand by the--set/--unsetvalidation error. Two commits, one per path.1.
config --list. AddsConfig.Redacted(), which returns a copy with sensitive values replaced by[REDACTED], and marshals that for output instead of the raw config. The plaintext renderer,--output json, and the JSON default used when stdout is not a terminal all derive from the samejson.Marshalincmd/config/config.go, so the single substitution covers all three.Two details worth flagging for review:
--list.[REDACTED], soomitemptystill elides the key. Reporting an absent token as "present but hidden" would be a worse answer than omitting it.I chose an explicit method over reflecting on a new struct tag, or over unmarshalling to a
mapand substituting there. The map route re-sorts the keys and changes output shape for everyone; the method preserves field order and matches the hand-writtenswitchalready used inUpdate.2. Validation errors.
--setand--unsetreject an unknown key by echoing it back. The argument in that position is not always a key — transposing--set <key> <value>puts the value there, and foraccess-tokenthat value is a secret:Rather than trying to detect secrets — a heuristic on token prefixes will rot the moment a token format changes — this checks the shape of what is about to be echoed. Configuration keys are short lowercase-hyphenated words; anything else is not a key, so there is no reason to reflect it back. The typo case that the echo exists to serve is unaffected:
Describe alternatives you've considered
--show-secretsflag. Deliberately omitted. It weakens the guarantee to "safe unless someone passes a flag", and a flag whose purpose is to print a secret tends to end up in scripts. No capability is lost without it: the config file is plain YAML at a documented path, soyq '.["access-token"]' "$XDG_CONFIG_HOME/ldcli/config.yml"still works for anyone who genuinely needs the value. Happy to add the flag if you would rather not take the behavior change unconditionally.api-••••••••cd12). Lets a human identify which token is stored, but still puts live secret characters into scrollback, andldcli whoamialready names the token without revealing it.cmd/cliflags. Attractive sinceAllFlagsHelp()is already the single authority for valid config keys, but promoting it frommap[string]stringto a struct changes a shape used by the publicconfig --helplisting, for no benefit at one sensitive key. Easy to move to later if a second one appears.Additional context
Behavior change, so calling it out plainly: any caller parsing the token out of
config --listoutput will now read[REDACTED]. I believe that is the point of the change rather than a regression, but it is your call whether it needs more than a patch release.cmd/config/testdata/help.goldenis unchanged — no flag is added and no description is edited.Tests added in
internal/config/config_test.go:Redacted()— token replaced when set; left empty when unset (asserted throughjson.Marshal, soomitemptyis actually exercised); non-sensitive fields untouched; receiver not mutated.TestRedactedOutput— marshals the redacted config throughoutput.CmdOutputSingularfor bothjsonandplaintext, asserting the token appears in neither. This is what pins sites 2 and 3 rather than just the struct.TestErrorDoesNotEchoNonKeyArguments— transposed--setand non-key--unsetredact; a key-shaped typo still echoes verbatim.I skipped an end-to-end
config --listtest on purpose: it needsviper.SetConfigFileon global state thatcmd/root.goowns, and a flaky test seemed worse than a missing one. I did verify the built binary by hand in all three output modes.Verified locally:
make build,go test ./...,go vet, andgolangci-lintv1.63.4 all clean;gofmt -lreports nothing in the touched files. (cmd/root.goandcmd/dev_server/projects.goare flagged by gofmt onmainalready — I left them alone rather than mix unrelated formatting into this diff.)Note
Overview
Stops the LaunchDarkly CLI from leaking the stored access token when users run
config --listor hit invalid-key errors onconfig --set/--unset.config --listnow JSON-marshalsConfig.Redacted()instead of the live struct, soaccess-tokenappears as[REDACTED]in JSON and plaintext output while the on-disk YAML is unchanged. Unset tokens stay empty soomitemptystill omits the field.Validation errors no longer echo arbitrary rejected “keys”:
safeKeyForErroronly repeats arguments that look like real config keys (short, lowercase-hyphenated); mistyped--setthat puts the token in the key slot shows[REDACTED]instead of the secret.Tests cover
Redacted(), both output modes viaCmdOutputSingular, and the error-path redaction behavior.Reviewed by Cursor Bugbot for commit b737ff2. Bugbot is set up for automated code reviews on this repo. Configure here.