Skip to content

Commit d30c2b3

Browse files
feat(config): store API keys in OS keychain instead of plaintext
API keys and OAuth tokens are now stored in the operating system's native keychain (macOS Keychain, Windows Credential Manager, Linux Secret Service) instead of plaintext in crush.json. Existing plaintext keys are automatically migrated to the keychain on first load. A placeholder marker (__keyring__) is written to the JSON config file so the config system still recognizes that a provider is configured. When the keychain is unavailable (headless servers, CI, containers), the system falls back to file-based storage with a warning. Set CRUSH_DISABLE_KEYRING=1 to force file-based storage. Closes #2477
1 parent 8b64369 commit d30c2b3

File tree

7 files changed

+326
-29
lines changed

7 files changed

+326
-29
lines changed

go.mod

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ require (
6161
github.com/stretchr/testify v1.11.1
6262
github.com/tidwall/gjson v1.18.0
6363
github.com/tidwall/sjson v1.2.5
64+
github.com/zalando/go-keyring v0.2.8
6465
github.com/zeebo/xxh3 v1.1.0
6566
go.uber.org/goleak v1.3.0
6667
golang.org/x/net v0.52.0
@@ -105,6 +106,7 @@ require (
105106
github.com/charmbracelet/x/json v0.2.0 // indirect
106107
github.com/charmbracelet/x/termios v0.1.1 // indirect
107108
github.com/charmbracelet/x/windows v0.2.2 // indirect
109+
github.com/danieljoos/wincred v1.2.3 // indirect
108110
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
109111
github.com/dlclark/regexp2 v1.11.5 // indirect
110112
github.com/ebitengine/purego v0.10.0 // indirect

go.sum

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,8 @@ github.com/cncf/xds/go v0.0.0-20260202195803-dba9d589def2/go.mod h1:qwXFYgsP6T7X
141141
github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g=
142142
github.com/creack/pty v1.1.24 h1:bJrF4RRfyJnbTJqzRLHzcGaZK1NeM5kTC9jGgovnR1s=
143143
github.com/creack/pty v1.1.24/go.mod h1:08sCNb52WyoAwi2QDyzUCTgcvVFhUzewun7wtTfvcwE=
144+
github.com/danieljoos/wincred v1.2.3 h1:v7dZC2x32Ut3nEfRH+vhoZGvN72+dQ/snVXo/vMFLdQ=
145+
github.com/danieljoos/wincred v1.2.3/go.mod h1:6qqX0WNrS4RzPZ1tnroDzq9kY3fu1KwE7MRLQK4X0bs=
144146
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
145147
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
146148
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM=
@@ -357,6 +359,8 @@ github.com/spf13/pflag v1.0.9/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An
357359
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
358360
github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
359361
github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
362+
github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY=
363+
github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA=
360364
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
361365
github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
362366
github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
@@ -392,6 +396,8 @@ github.com/yuin/goldmark v1.7.8 h1:iERMLn0/QJeHFhxSt3p6PeN9mGnvIKSpG9YYorDMnic=
392396
github.com/yuin/goldmark v1.7.8/go.mod h1:uzxRWxtg69N339t3louHJ7+O03ezfj6PlliRlaOzY1E=
393397
github.com/yuin/goldmark-emoji v1.0.5 h1:EMVWyCGPlXJfUXBXpuMu+ii3TIaxbVBnEX9uaDC4cIk=
394398
github.com/yuin/goldmark-emoji v1.0.5/go.mod h1:tTkZEbwu5wkPmgTcitqddVxY9osFZiavD+r4AzQrh1U=
399+
github.com/zalando/go-keyring v0.2.8 h1:6sD/Ucpl7jNq10rM2pgqTs0sZ9V3qMrqfIIy5YPccHs=
400+
github.com/zalando/go-keyring v0.2.8/go.mod h1:tsMo+VpRq5NGyKfxoBVjCuMrG47yj8cmakZDO5QGii0=
395401
github.com/zeebo/assert v1.3.0 h1:g7C04CbJuIDKNPFHmsk4hwZDO5O+kntRxzaUoNXj+IQ=
396402
github.com/zeebo/assert v1.3.0/go.mod h1:Pq9JiuJQpG8JLJdtkwrJESF0Foym2/D9XMU5ciN/wJ0=
397403
github.com/zeebo/xxh3 v1.1.0 h1:s7DLGDK45Dyfg7++yxI0khrfwq9661w9EN78eP/UZVs=

internal/cmd/login.go

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
package cmd
22

33
import (
4-
"cmp"
54
"context"
65
"fmt"
76
"os"
87
"os/signal"
98

9+
"charm.land/catwalk/pkg/catwalk"
1010
"charm.land/lipgloss/v2"
1111
"github.com/atotto/clipboard"
1212
hyperp "github.com/charmbracelet/crush/internal/agent/hyper"
@@ -111,10 +111,7 @@ func loginHyper(cfg *config.ConfigStore) error {
111111
return fmt.Errorf("access token is not active")
112112
}
113113

114-
if err := cmp.Or(
115-
cfg.SetConfigField(config.ScopeGlobal, "providers.hyper.api_key", token.AccessToken),
116-
cfg.SetConfigField(config.ScopeGlobal, "providers.hyper.oauth", token),
117-
); err != nil {
114+
if err := cfg.SetProviderAPIKey(config.ScopeGlobal, "hyper", token); err != nil {
118115
return err
119116
}
120117

@@ -126,7 +123,8 @@ func loginHyper(cfg *config.ConfigStore) error {
126123
func loginCopilot(cfg *config.ConfigStore) error {
127124
ctx := getLoginContext()
128125

129-
if cfg.HasConfigField(config.ScopeGlobal, "providers.copilot.oauth") {
126+
if cfg.HasConfigField(config.ScopeGlobal, "providers.copilot.oauth") ||
127+
cfg.HasConfigField(config.ScopeGlobal, "providers.copilot.api_key") {
130128
fmt.Println("You are already logged in to GitHub Copilot.")
131129
return nil
132130
}
@@ -176,10 +174,7 @@ func loginCopilot(cfg *config.ConfigStore) error {
176174
token = t
177175
}
178176

179-
if err := cmp.Or(
180-
cfg.SetConfigField(config.ScopeGlobal, "providers.copilot.api_key", token.AccessToken),
181-
cfg.SetConfigField(config.ScopeGlobal, "providers.copilot.oauth", token),
182-
); err != nil {
177+
if err := cfg.SetProviderAPIKey(config.ScopeGlobal, string(catwalk.InferenceProviderCopilot), token); err != nil {
183178
return err
184179
}
185180

internal/config/keyring.go

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
package config
2+
3+
import (
4+
"encoding/json"
5+
"fmt"
6+
"log/slog"
7+
"os"
8+
"strings"
9+
10+
"github.com/charmbracelet/crush/internal/oauth"
11+
"github.com/zalando/go-keyring"
12+
)
13+
14+
const (
15+
// keyringService is the service name used in the OS keychain.
16+
keyringService = "crush"
17+
18+
// KeyringMarker is the sentinel value stored in the JSON config file to
19+
// indicate that the real credential lives in the OS keychain.
20+
KeyringMarker = "__keyring__"
21+
)
22+
23+
// KeyringStore provides secure credential storage via the OS keychain
24+
// (macOS Keychain, Windows Credential Manager, Linux Secret Service / D-Bus).
25+
// When the keychain is unavailable (headless servers, CI, etc.) it reports
26+
// itself as unavailable so callers can fall back to file-based storage.
27+
type KeyringStore struct {
28+
available bool
29+
}
30+
31+
// NewKeyringStore creates a KeyringStore and probes the system keychain for
32+
// availability. Set CRUSH_DISABLE_KEYRING=1 to force file-based storage.
33+
func NewKeyringStore() *KeyringStore {
34+
ks := &KeyringStore{}
35+
36+
if v := os.Getenv("CRUSH_DISABLE_KEYRING"); v == "1" || strings.EqualFold(v, "true") {
37+
slog.Debug("OS keychain disabled via CRUSH_DISABLE_KEYRING")
38+
return ks
39+
}
40+
41+
// Probe the keyring with a harmless set/delete cycle.
42+
const probe = "crush-keyring-probe"
43+
if err := keyring.Set(keyringService, probe, "probe"); err != nil {
44+
slog.Warn("OS keychain is not available, credentials will be stored in config file", "error", err)
45+
return ks
46+
}
47+
_ = keyring.Delete(keyringService, probe)
48+
ks.available = true
49+
slog.Debug("OS keychain is available")
50+
return ks
51+
}
52+
53+
// Available reports whether the OS keychain is accessible.
54+
func (ks *KeyringStore) Available() bool {
55+
return ks != nil && ks.available
56+
}
57+
58+
// --- API key helpers ---
59+
60+
func apiKeyName(providerID string) string {
61+
return providerID + "/api_key"
62+
}
63+
64+
// SetAPIKey stores an API key in the OS keychain.
65+
func (ks *KeyringStore) SetAPIKey(providerID, apiKey string) error {
66+
if !ks.Available() {
67+
return fmt.Errorf("keyring not available")
68+
}
69+
return keyring.Set(keyringService, apiKeyName(providerID), apiKey)
70+
}
71+
72+
// GetAPIKey retrieves an API key from the OS keychain.
73+
func (ks *KeyringStore) GetAPIKey(providerID string) (string, error) {
74+
if !ks.Available() {
75+
return "", fmt.Errorf("keyring not available")
76+
}
77+
return keyring.Get(keyringService, apiKeyName(providerID))
78+
}
79+
80+
// DeleteAPIKey removes an API key from the OS keychain.
81+
func (ks *KeyringStore) DeleteAPIKey(providerID string) error {
82+
if !ks.Available() {
83+
return fmt.Errorf("keyring not available")
84+
}
85+
return keyring.Delete(keyringService, apiKeyName(providerID))
86+
}
87+
88+
// --- OAuth token helpers ---
89+
90+
func oauthKeyName(providerID string) string {
91+
return providerID + "/oauth"
92+
}
93+
94+
// SetOAuthToken stores a serialized OAuth token in the OS keychain.
95+
func (ks *KeyringStore) SetOAuthToken(providerID string, token *oauth.Token) error {
96+
if !ks.Available() {
97+
return fmt.Errorf("keyring not available")
98+
}
99+
data, err := json.Marshal(token)
100+
if err != nil {
101+
return fmt.Errorf("failed to marshal OAuth token: %w", err)
102+
}
103+
return keyring.Set(keyringService, oauthKeyName(providerID), string(data))
104+
}
105+
106+
// GetOAuthToken retrieves an OAuth token from the OS keychain.
107+
func (ks *KeyringStore) GetOAuthToken(providerID string) (*oauth.Token, error) {
108+
if !ks.Available() {
109+
return nil, fmt.Errorf("keyring not available")
110+
}
111+
data, err := keyring.Get(keyringService, oauthKeyName(providerID))
112+
if err != nil {
113+
return nil, err
114+
}
115+
var token oauth.Token
116+
if err := json.Unmarshal([]byte(data), &token); err != nil {
117+
return nil, fmt.Errorf("failed to unmarshal OAuth token from keyring: %w", err)
118+
}
119+
return &token, nil
120+
}
121+
122+
// DeleteOAuthToken removes an OAuth token from the OS keychain.
123+
func (ks *KeyringStore) DeleteOAuthToken(providerID string) error {
124+
if !ks.Available() {
125+
return fmt.Errorf("keyring not available")
126+
}
127+
return keyring.Delete(keyringService, oauthKeyName(providerID))
128+
}
129+
130+
// --- Helpers ---
131+
132+
// IsKeyringMarker reports whether a config value is the keyring placeholder.
133+
func IsKeyringMarker(value string) bool {
134+
return value == KeyringMarker
135+
}
136+
137+
// isEnvVarReference reports whether a value looks like an environment variable
138+
// reference (e.g. $OPENAI_API_KEY or ${OPENAI_API_KEY}).
139+
func isEnvVarReference(value string) bool {
140+
return strings.HasPrefix(value, "$")
141+
}
142+
143+
// isCommandSubstitution reports whether a value looks like a command
144+
// substitution (e.g. $(some-command)).
145+
func isCommandSubstitution(value string) bool {
146+
return strings.HasPrefix(value, "$(") && strings.HasSuffix(value, ")")
147+
}
148+
149+
// isLiteralSecret reports whether a value is a plaintext literal secret
150+
// (not a marker, not an env-var reference, not a command substitution, and not empty).
151+
func isLiteralSecret(value string) bool {
152+
return value != "" &&
153+
!IsKeyringMarker(value) &&
154+
!isEnvVarReference(value) &&
155+
!isCommandSubstitution(value)
156+
}

internal/config/load.go

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ func Load(workingDir, dataDir string, debug bool) (*ConfigStore, error) {
4646
workingDir: workingDir,
4747
globalDataPath: GlobalConfigData(),
4848
workspacePath: filepath.Join(cfg.Options.DataDirectory, fmt.Sprintf("%s.json", appName)),
49+
keyring: NewKeyringStore(),
4950
}
5051

5152
if debug {
@@ -168,6 +169,29 @@ func (c *Config) configureProviders(store *ConfigStore, env env.Env, resolver Va
168169
config, configExists := c.Providers.Get(string(p.ID))
169170
// if the user configured a known provider we need to allow it to override a couple of parameters
170171
if configExists {
172+
// Resolve credentials from keyring when the marker is present.
173+
if IsKeyringMarker(config.APIKey) {
174+
if store.keyring.Available() {
175+
if realKey, err := store.keyring.GetAPIKey(string(p.ID)); err == nil {
176+
config.APIKey = realKey
177+
} else {
178+
slog.Warn("Failed to read API key from OS keychain", "provider", p.ID, "error", err)
179+
config.APIKey = "" // Clear marker so provider is properly skipped
180+
}
181+
if token, err := store.keyring.GetOAuthToken(string(p.ID)); err == nil {
182+
config.OAuthToken = token
183+
}
184+
} else {
185+
slog.Warn("Config has keyring marker but OS keychain is unavailable", "provider", p.ID)
186+
config.APIKey = "" // Clear marker — can't resolve without keychain
187+
}
188+
}
189+
190+
// Migrate plaintext literal keys to keyring on first load.
191+
if isLiteralSecret(config.APIKey) && store.keyring.Available() {
192+
migrateProviderToKeyring(store, string(p.ID), &config)
193+
}
194+
171195
if config.BaseURL != "" {
172196
p.APIEndpoint = config.BaseURL
173197
}
@@ -308,6 +332,29 @@ func (c *Config) configureProviders(store *ConfigStore, env env.Env, resolver Va
308332
continue
309333
}
310334

335+
// Resolve credentials from keyring when the marker is present.
336+
if IsKeyringMarker(providerConfig.APIKey) {
337+
if store.keyring.Available() {
338+
if realKey, err := store.keyring.GetAPIKey(id); err == nil {
339+
providerConfig.APIKey = realKey
340+
} else {
341+
slog.Warn("Failed to read API key from OS keychain", "provider", id, "error", err)
342+
providerConfig.APIKey = "" // Clear marker so provider is properly skipped
343+
}
344+
if token, err := store.keyring.GetOAuthToken(id); err == nil {
345+
providerConfig.OAuthToken = token
346+
}
347+
} else {
348+
slog.Warn("Config has keyring marker but OS keychain is unavailable", "provider", id)
349+
providerConfig.APIKey = "" // Clear marker — can't resolve without keychain
350+
}
351+
}
352+
353+
// Migrate plaintext literal keys to keyring on first load.
354+
if isLiteralSecret(providerConfig.APIKey) && store.keyring.Available() {
355+
migrateProviderToKeyring(store, id, &providerConfig)
356+
}
357+
311358
// Make sure the provider ID is set
312359
providerConfig.ID = id
313360
providerConfig.Name = cmp.Or(providerConfig.Name, id) // Use ID as name if not set
@@ -816,3 +863,37 @@ func GlobalSkillsDirs() []string {
816863
}
817864

818865
func isAppleTerminal() bool { return os.Getenv("TERM_PROGRAM") == "Apple_Terminal" }
866+
867+
// migrateProviderToKeyring moves a plaintext API key (and optional OAuth token)
868+
// from the JSON config file into the OS keychain, replacing the values in the
869+
// file with markers. The in-memory config retains the real values.
870+
func migrateProviderToKeyring(store *ConfigStore, providerID string, pc *ProviderConfig) {
871+
if !store.keyring.Available() {
872+
return
873+
}
874+
875+
if err := store.keyring.SetAPIKey(providerID, pc.APIKey); err != nil {
876+
slog.Warn("Migration: failed to store API key in keychain", "provider", providerID, "error", err)
877+
return
878+
}
879+
880+
// Replace plaintext in JSON with marker.
881+
if err := store.SetConfigField(ScopeGlobal, fmt.Sprintf("providers.%s.api_key", providerID), KeyringMarker); err != nil {
882+
slog.Warn("Migration: failed to write keyring marker", "provider", providerID, "error", err)
883+
return
884+
}
885+
886+
// Migrate OAuth token too if present.
887+
if pc.OAuthToken != nil {
888+
if err := store.keyring.SetOAuthToken(providerID, pc.OAuthToken); err != nil {
889+
slog.Warn("Migration: failed to store OAuth token in keychain", "provider", providerID, "error", err)
890+
return
891+
}
892+
if err := store.SetConfigField(ScopeGlobal, fmt.Sprintf("providers.%s.oauth", providerID), KeyringMarker); err != nil {
893+
slog.Warn("Migration: failed to write OAuth keyring marker", "provider", providerID, "error", err)
894+
return
895+
}
896+
}
897+
898+
slog.Info("Migrated credentials from config file to OS keychain", "provider", providerID)
899+
}

0 commit comments

Comments
 (0)