Skip to content

Store auth credentials in the OS keychain (macOS + Linux)#2077

Open
ashb wants to merge 6 commits into
mainfrom
store-creds-in-secure-os-backed-store
Open

Store auth credentials in the OS keychain (macOS + Linux)#2077
ashb wants to merge 6 commits into
mainfrom
store-creds-in-secure-os-backed-store

Conversation

@ashb
Copy link
Copy Markdown
Contributor

@ashb ashb commented Apr 9, 2026

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.

After building and running this, the config file looks like:

Screenshot 2026-04-09 at 13 11 54

And we can see the item in keychain access.

Screenshot 2026-04-09 at 13 09 10

As of this Pr. astro auth token still 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)

ashb added 2 commits April 9, 2026 13:06
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.
@coveralls-official
Copy link
Copy Markdown

coveralls-official Bot commented Apr 9, 2026

Coverage Report for CI Build 0

Coverage increased (+0.04%) to 39.388%

Details

  • Coverage increased (+0.04%) from the base build.
  • Patch coverage: 136 uncovered changes across 24 files (367 of 503 lines covered, 72.96%).
  • 40 coverage regressions across 6 files.

Uncovered Changes

Top 10 Files by Coverage Impact Changed Covered %
pkg/keychain/keychain_file.go 73 51 69.86%
cmd/root_hooks.go 41 21 51.22%
cmd/cloud/setup.go 28 16 57.14%
pkg/keychain/keychain.go 31 22 70.97%
cmd/software/deployment_logs.go 8 0 0.0%
cloud/auth/auth.go 20 13 65.0%
software/auth/auth.go 18 11 61.11%
cloud/deployment/deployment.go 16 10 62.5%
config/context.go 31 26 83.87%
airflow/docker.go 11 7 63.64%

Coverage Regressions

40 previously-covered lines in 6 files lost coverage.

File Lines Losing Coverage Coverage
cmd/cloud/setup.go 32 61.59%
software/auth/auth.go 3 83.17%
cloud/auth/auth.go 2 85.88%
cmd/api/api.go 1 82.98%
cmd/root_hooks.go 1 58.93%
config/context.go 1 80.0%

Coverage Stats

Coverage Status
Relevant Lines: 63346
Covered Lines: 24951
Line Coverage: 39.39%
Coverage Strength: 9.5 hits per line

💛 - Coveralls

@ashb ashb force-pushed the store-creds-in-secure-os-backed-store branch 2 times, most recently from 30f1757 to b8fab78 Compare April 9, 2026 14:17
@ashb ashb force-pushed the store-creds-in-secure-os-backed-store branch from b8fab78 to f1c29a5 Compare April 9, 2026 14:49
Copy link
Copy Markdown
Contributor

@neel-astro neel-astro left a comment

Choose a reason for hiding this comment

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

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

Comment thread cloud/deploy/bundle.go Outdated
Comment thread pkg/keychain/keychain_linux.go Outdated
Comment thread software/auth/auth.go Outdated
ashb and others added 2 commits April 10, 2026 14:09
- 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>
@ashb ashb marked this pull request as ready for review April 14, 2026 10:22
@ashb ashb requested review from a team as code owners April 14, 2026 10:22
@ashb
Copy link
Copy Markdown
Contributor Author

ashb commented Apr 14, 2026

I'm testing/asking for testers with this on APC before we merge.

It was missing setting the current credentials, so it failed to verify the
token in the post login step
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.

2 participants