Skip to content

fix(config): refuse plaintext credentials and redact them in config list - #46

Merged
timimsms merged 1 commit into
mainfrom
fix/refuse-plaintext-credentials
Aug 28, 2026
Merged

timimsms merged 1 commit into
mainfrom
fix/refuse-plaintext-credentials

Conversation

@timimsms

Copy link
Copy Markdown
Owner

Summary

cu config set api_token <token> wrote the token to ~/.config/cu/config.yaml in plaintext — and then never used it, because authentication reads the OS keyring only:

$ cu config set api_token sk-PLAINTEXT-EXAMPLE
Set api_token to sk-PLAINTEXT-EXAMPLE
$ cat ~/.config/cu/config.yaml
api_token: sk-PLAINTEXT-EXAMPLE
$ cu auth status
Not authenticated

The operation had no upside at all: it leaked a secret to disk and left the user unauthenticated. Nothing in the codebase reads api_token back — internal/config declares the field, and that is the only reference.

This is an asymmetry I introduced in #43. That PR added a credential filter for project .cu.yml files but Set staged every key, so the global config had no equivalent guard.

Changes

  • cu config set refuses credential keys and points at cu auth login, exiting non-zero.
  • config.Set no longer stages them, as a backstop. They still apply in-process, so anything relying on a runtime override is unaffected — but they can never reach disk.
  • cu config list redacts credential values. That output gets pasted into issues and terminals far more casually than an explicit config get <key>, which is left alone as a deliberate act by the user. Happy to redact get too if you'd rather be uniform.
  • config.IsCredentialKey is exported so both layers share one definition.
$ cu config set api_token sk-NEW
Refusing to write "api_token" to the config file — it would be stored in plaintext and never used.
Authenticate with 'cu auth login' instead; the token is kept in your system keyring.
$ echo $?
1
$ cu config list | grep api_token
api_token=<redacted — cu authenticates via the system keyring, not this file>

A token already in a config file is preserved, not silently deleted. Removing a user's data on the next unrelated config set would be a worse surprise than leaving it; refusing to add more is the fix, and list redacts what is already there. The redaction text deliberately does not claim the value lives in the keyring, because for a legacy entry it does not.

Tests

TestCredentialKeysAreNeverStaged covers the key predicate, that Set applies in-process while never writing to disk, and that a pre-existing plaintext token survives an unrelated save.

Checklist

  • ./scripts/ci.sh passes locally — except errcheck, which reports the same pre-existing findings on main, none in files this PR touches
  • Commit messages use conventional prefixes
  • CLI docs regenerated if command help text changed — no help text changed
  • Docs updated if user-facing behavior changed

`cu config set api_token <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 <key>`, 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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014aqbmccWm1tqttmBUCR5rv
ClickUp: 86dxbeqyt
@timimsms

Copy link
Copy Markdown
Owner Author

Review — approve, no blockers

Verified every claim in the description against a built binary with a throwaway HOME, including a legacy api_token: legacy-plaintext-token already on disk:

$ cu config set api_token sk-NEW
Refusing to write "api_token" to the config file — it would be stored in plaintext and never used.
Authenticate with 'cu auth login' instead; the token is kept in your system keyring.
exit=1
$ cu config set API_TOKEN sk-NEW2      # case-insensitive
exit=1
$ cu config set default_list abc123    # ordinary keys unaffected
exit=0
$ cu config list
api_token=<redacted — cu authenticates via the system keyring, not this file>
default_list=abc123
$ cat ~/.config/cu/config.yaml
api_token: legacy-plaintext-token      # preserved, as designed
default_list: abc123

Also confirmed the premise independently on this branch — api_token appears exactly twice in non-test code, the mapstructure tag and the credentialKeys entry itself. Nothing reads it back, so refusing the write costs the user nothing.

Two calls I think are right and worth keeping:

Preserving a pre-existing token rather than scrubbing it. Deleting user data during an unrelated config set would be the worse surprise, and the test pins it.

The redaction string not claiming the value lives in the keyring. For a legacy entry that would be a lie, and it's the exact case where the text is displayed.

Non-blocking: one asymmetry is left

SaveProjectConfig still writes its settings map to .cu.yml verbatim, with no IsCredentialKey filter. So the credential story now reads: refused on read from .cu.yml (#43), refused on write to the global config (this PR), permitted on write to .cu.yml — where Init would then refuse to read it back, leaving a plaintext token on disk that nothing will ever use. Exactly the state this PR exists to prevent.

It's latent, not live: the only caller is list.go:175, passing default_list. A three-line guard would close it, or it can ride along with the config get follow-up below. Either is fine — not worth holding this PR.

On your config get question

I'd redact it too. The argument for leaving it alone — that get <key> names the exact key, so it's deliberate — is real but doesn't survive the threat model. What redaction actually defends against is incidental disclosure: pasted terminal output, a screen-share, a CI log. cu config get api_token inside a script that logs its output is precisely that case, and it's the shape most likely to end up in a log file.

The strongest counter-argument is recovery: this PR deliberately preserves a legacy token, and redacting get makes the preserved value unreachable through the tool. But it's plaintext YAML at a documented path — cat ~/.config/cu/config.yaml is right there, and the --config help now points at it (#47). Redaction never protects the secret from its owner; it only ever protects against the accidental audience. So the asymmetry buys very little and costs the uniform rule that's easier to reason about.

One real breakage to weigh: a legacy TOKEN=$(cu config get api_token) in someone's script would start capturing the redaction string. That's a genuine regression, though a narrow one — config set api_token is now refused, so no new user can reach that state, and for existing ones the captured token was never one cu itself used.

Suggestion if you take it: have the redaction name the config file path, so an interactive user immediately knows where to look, and pair it with the SaveProjectConfig guard in one small follow-up. Neither needs to hold v0.2.1.

@timimsms
timimsms merged commit d0351cb into main Aug 28, 2026
16 checks passed
@timimsms
timimsms deleted the fix/refuse-plaintext-credentials branch August 28, 2026 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant