Skip to content

Conversation

@mcncl
Copy link
Contributor

@mcncl mcncl commented Nov 11, 2025

Description

Currently we store API tokens in plain text in a config file in the user's .config directory, that's not ideal.

Changes

This change moves to using keychain (or Linux/Windows variants) for API tokens by default, but users can override this by setting the token source as file using an environment variable.

We've implemented the following;

  • bk configure add still prompts as normal, but on storing the API token we use the organization as the identifier within keychain
  • provides a bk configure migrate command to move existing tokens and their organization to keychain
  • removes the use of a ~/.config/bk.yaml file entirely
  • tests included for this change (all passing)

@mcncl mcncl requested a review from a team as a code owner November 11, 2025 23:05
@mcncl mcncl changed the title feat: use keychain for api token SUP-5203: use keychain for api token Nov 11, 2025
Copy link
Contributor

@JoeColeman95 JoeColeman95 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - Only thing is I'd put something about the keychain org list limitation in the heredoc somewhere

@scadu
Copy link
Contributor

scadu commented Nov 12, 2025

Some folks use the CLI in CI pipelines, so we definitely want to mention that as a breaking chance, since keyring would be the default now.

@mcncl
Copy link
Contributor Author

mcncl commented Nov 12, 2025

@scadu it should be falling back to the config if the token is not present in keychain

Comment on lines 121 to +124
// APIToken gets the API token configured for the currently selected organization
// Priority order: BUILDKITE_API_TOKEN env var → keychain → config file (legacy)
func (conf *Config) APIToken() string {
// Environment variable takes precedence
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@scadu this section should be what you're after

Copy link
Contributor

@scadu scadu Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mcncl ah, thanks for pointing that! Nice!

@mcncl
Copy link
Contributor Author

mcncl commented Nov 12, 2025

@JoeColeman95 good point; bk use won't work with this if a user does have more than one org, they'd need to run bk configure add and then provide their org which already exists in Keychain for it to then "use" that org

I'll work on a fix; storing orgs as keys with empty values in the config file so the list of user orgs is still present and can be used as a look up against Keychain

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.

4 participants