Store auth credentials in the OS keychain (macOS + Linux)#2077
Draft
Store auth credentials in the OS keychain (macOS + Linux)#2077
Conversation
Move auth tokens and refresh tokens out of the plaintext config.yaml into the OS-native credential store via 99designs/keyring: - macOS: Keychain Services with per-app ACL (re-prompts after binary updates when another process tries to read the item) - Linux: Secret Service (GNOME Keyring / KWallet) when a D-Bus session is available - Windows support to follow Credentials are managed through a SecureStore interface (pkg/keychain) with Get/Set/DeleteCredentials methods. A TokenHolder (pkg/httputil) is populated once during PersistentPreRunE and read by API client request editors on every outbound request — replacing the previous per-request context lookup for the token. On first run, MigrateLegacyCredentials copies token/refreshtoken fields from existing config.yaml contexts into the secure store and clears them from the config file. On Linux, when no Secret Service daemon is reachable (headless CI, containers without D-Bus), the store falls back to a plaintext JSON file at ~/.astro/credentials.json (mode 0600). This matches the previous config.yaml security posture and is acceptable because CI environments typically use ASTRO_API_TOKEN rather than interactive login credentials. Windows uses the same plaintext file fallback for now; the Credential Manager backend is added in the follow-up commit.
Coverage Report for CI Build 609Coverage increased (+0.04%) to 39.387%Details
Uncovered Changes
Coverage Regressions43 previously-covered lines in 7 files lost coverage.
Coverage Stats
💛 - Coveralls |
ashb
force-pushed
the
store-creds-in-secure-os-backed-store
branch
2 times, most recently
from
30f1757 to
b8fab78
Compare
ashb
force-pushed
the
store-creds-in-secure-os-backed-store
branch
from
b8fab78 to
f1c29a5
Compare
neel-astro
reviewed
Apr 10, 2026
Contributor
neel-astro
left a comment
neel-astro
left a comment
There was a problem hiding this comment.
Mostly looks great apart from tiny comments, this would be a great addition to CLI posture, so thanks for all the effort 🙇
Also, it would be worth checking basic functionality with APC platform or ask APC team to check the same
cloud/deploy/bundle.go
Outdated
|
|
||
| // if CI/CD is enforced, check the subject can deploy | ||
| if currentDeployment.IsCicdEnforced && !canCiCdDeploy(c.Token) { | ||
| if currentDeployment.IsCicdEnforced && !canCiCdDeploy("Bearer "+os.Getenv("ASTRO_API_TOKEN")) { |
Contributor
There was a problem hiding this comment.
curious q: why do we directly refer api token env var here?
Edit: Ahh, because CI/CD deploy mode can only use api token and not session tokens, can we simplify this fetching env var logic a bit into some method, because it is being used in bunch of places, and I think we should error out early with appropriate error message if env var is not set, rather than passing empty value to canCiCdDeploy and failing on parsing token
pkg/keychain/keychain_linux.go
Outdated
Comment on lines
+19
to
+22
| // NOTE: if 99designs/keyring fails to connect to Secret Service in environments | ||
| // that DO have it running, replace with godbus/dbus directly: | ||
| // https://github.com/godbus/dbus — the SecureStore interface is the only | ||
| // change boundary. |
Contributor
There was a problem hiding this comment.
Is this a comment for future devs or just for you while you are running through tests in various envs
Contributor
Author
There was a problem hiding this comment.
Yeah not really relevant anymore, was a note to myself. Will remove.
software/auth/auth.go
Outdated
| if err != nil { | ||
| return | ||
| func Logout(domain string, store keychain.SecureStore) { | ||
| if err := store.DeleteCredentials(domain); err != nil { |
Contributor
There was a problem hiding this comment.
nit: probably need the store == nil check here as well, similar to what we have on the cloud side?
- Add tests for cachedStore, fileStore, loadSoftwareToken, and linux New() - Restrict Linux keyring backends to SecretService/KWallet only (keyctl doesn't persist across reboots, pass/file prompt for passphrases) - Remove stale NOTE comment from keychain_linux.go - Add store==nil guard to software Logout (reachable via login/logout which skip the storeErr check in pre-run hook) - Ignore 0o600/0o700 file permission literals in golangci-lint mnd Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
2 participants
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.
Move auth tokens and refresh tokens out of the plaintext config.yaml
into the OS-native credential store via 99designs/keyring:
updates when another process tries to read the item)
session is available
Credentials are managed through a SecureStore interface (pkg/keychain)
with Get/Set/DeleteCredentials methods. A TokenHolder (pkg/httputil) is
populated once during PersistentPreRunE and read by API client request
editors on every outbound request — replacing the previous per-request
context lookup for the token.
On first run, MigrateLegacyCredentials copies token/refreshtoken fields
from existing config.yaml contexts into the secure store and clears
them from the config file.
On Linux, when no Secret Service daemon is reachable (headless CI,
containers without D-Bus), the store falls back to a plaintext JSON
file at ~/.astro/credentials.json (mode 0600). This matches the
previous config.yaml security posture and is acceptable because CI
environments typically use ASTRO_API_TOKEN rather than interactive
login credentials.
Windows uses the same plaintext file fallback for now; the Credential
Manager backend is added in the follow-up commit.
After building and running this, the config file looks like:
And we can see the item in keychain access.
As of this Pr.
astro auth tokenstill works without further prompting (but I have a separate PR to put that, at least on macOS behind a TouchID et al prompt).A similar approach is possible on Windows (and the code is written, but not yet tested)