Merge pull request #34 from priyanshujain/cross-platform-keyring

refactor(provider): use go-keyring for cross-platform credential storage

Author: priyanshujain

docs/credential-storage.md

@@ -0,0 +1,50 @@
+# Credential Storage
+
+## How it works
+
+API keys are stored in the OS-native keyring via [`zalando/go-keyring`](https://github.com/zalando/go-keyring):
+
+| Platform | Backend |
+|----------|---------|
+| macOS | Keychain |
+| Linux | Secret Service (GNOME Keyring / KDE Wallet) |
+| Windows | Credential Manager (WinCred) |
+
+For headless/CI/Docker environments where no keyring daemon is available, use environment variables (`ANTHROPIC_API_KEY`, `OPENAI_API_KEY`, `GEMINI_API_KEY`). `ResolveAPIKey` checks the keyring first, then falls back to env vars.
+
+## Credential ref format
+
+Stored in config as `api_key_ref: "keychain:obk/<provider>"`. The `keychain:` prefix is a historical name — it works on all platforms, not just macOS Keychain.
+
+## Design decisions and learnings
+
+### Why keyring-only, no file fallback
+
+Early versions silently fell back to plain-text files (`~/.obk/secrets/`) when the keyring was unavailable. We removed this after researching how production CLI tools handle it:
+
+- **GitHub CLI (`gh`)** does the same silent file fallback and [their team now considers it a mistake](https://github.com/cli/cli/issues/10108). Users unknowingly store tokens in plain text.
+- **aws-vault** never auto-falls back. Users must explicitly choose a backend (`--backend=file`).
+- **docker-credential-helpers** errors out if the configured helper fails.
+
+**The pattern:** Keyring succeeds or errors. Env vars cover headless. No silent degradation to insecure storage.
+
+### Why `zalando/go-keyring` over `99designs/keyring`
+
+- Pure Go on all platforms — works with `CGO_ENABLED=0` for cross-compilation
+- Simple API (`Get`/`Set`/`Delete`) — we don't need backend selection, encrypted file fallback, or kwallet/pass/keyctl support
+- Same library used by `gh` (GitHub CLI) and `chezmoi`
+- `99designs/keyring` is more powerful but pulls in more dependencies and complexity (backend selection, encrypted file backend, passphrase prompts) that we don't need
+
+### Why errors propagate directly
+
+Keyring errors (locked keychain, unsupported platform, data too big) are returned to the caller. This means:
+
+- Users see the real problem instead of a misleading "file not found" from a hidden fallback
+- `obk setup` fails visibly if the keyring isn't working, rather than silently writing secrets to disk
+- Debugging is straightforward — the error message tells you what happened
+
+### Backward compatibility
+
+- `go-keyring` can read credentials stored by the old hand-rolled `security` CLI code on macOS (it checks for encoding prefixes and returns raw values as-is)
+- New credentials stored by `go-keyring` are base64-encoded in the keychain — this is a one-way migration (no downgrade path, which is fine)
+- The `keychain:` ref prefix in config files is preserved

go.mod

@@ -10,6 +10,7 @@ require (
 	github.com/riverqueue/river/riverdriver/riversqlite v0.31.0
 	github.com/riverqueue/river/rivertype v0.31.0
 	github.com/spf13/cobra v1.9.1
+	github.com/zalando/go-keyring v0.2.6
 	go.mau.fi/whatsmeow v0.0.0-20260227112304-c9652e4448a2
 	golang.org/x/oauth2 v0.35.0
 	golang.org/x/time v0.14.0
@@ -20,6 +21,7 @@ require (
 )
 
 require (
+	al.essio.dev/pkg/shellescape v1.5.1 // indirect
 	cloud.google.com/go/auth v0.18.2 // indirect
 	cloud.google.com/go/auth/oauth2adapt v0.2.8 // indirect
 	cloud.google.com/go/compute/metadata v0.9.0 // indirect
@@ -38,13 +40,15 @@ require (
 	github.com/charmbracelet/x/exp/strings v0.0.0-20240722160745-212f7b056ed0 // indirect
 	github.com/charmbracelet/x/term v0.2.1 // indirect
 	github.com/coder/websocket v1.8.14 // indirect
+	github.com/danieljoos/wincred v1.2.2 // indirect
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/dustin/go-humanize v1.0.1 // indirect
 	github.com/elliotchance/orderedmap/v3 v3.1.0 // indirect
 	github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f // indirect
 	github.com/felixge/httpsnoop v1.0.4 // indirect
 	github.com/go-logr/logr v1.4.3 // indirect
 	github.com/go-logr/stdr v1.2.2 // indirect
+	github.com/godbus/dbus/v5 v5.1.0 // indirect
 	github.com/google/s2a-go v0.1.9 // indirect
 	github.com/google/uuid v1.6.0 // indirect
 	github.com/googleapis/enterprise-certificate-proxy v0.3.12 // indirect

go.sum

@@ -1,3 +1,5 @@
+al.essio.dev/pkg/shellescape v1.5.1 h1:86HrALUujYS/h+GtqoB26SBEdkWfmMI6FubjXlsXyho=
+al.essio.dev/pkg/shellescape v1.5.1/go.mod h1:6sIqp7X2P6mThCQ7twERpZTuigpr6KbZWtls1U8I890=
 cloud.google.com/go/auth v0.18.2 h1:+Nbt5Ev0xEqxlNjd6c+yYUeosQ5TtEUaNcN/3FozlaM=
 cloud.google.com/go/auth v0.18.2/go.mod h1:xD+oY7gcahcu7G2SG2DsBerfFxgPAJz17zz2joOFF3M=
 cloud.google.com/go/auth/oauth2adapt v0.2.8 h1:keo8NaayQZ6wimpNSmW5OPc283g65QNIiLpZnkHRbnc=
@@ -60,6 +62,8 @@ github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSV
 github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g=
 github.com/creack/pty v1.1.24 h1:bJrF4RRfyJnbTJqzRLHzcGaZK1NeM5kTC9jGgovnR1s=
 github.com/creack/pty v1.1.24/go.mod h1:08sCNb52WyoAwi2QDyzUCTgcvVFhUzewun7wtTfvcwE=
+github.com/danieljoos/wincred v1.2.2 h1:774zMFJrqaeYCK2W57BgAem/MLi6mtSE47MB6BOJ0i0=
+github.com/danieljoos/wincred v1.2.2/go.mod h1:w7w4Utbrz8lqeMbDAK0lkNJUv5sAOkFi7nd/ogr0Uh8=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
@@ -77,6 +81,8 @@ github.com/go-logr/logr v1.4.3/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ4
 github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag=
 github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE=
 github.com/godbus/dbus/v5 v5.0.4/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA=
+github.com/godbus/dbus/v5 v5.1.0 h1:4KLkAxT3aOY8Li4FRJe/KvhoNFFxo0m6fNuFUO8QJUk=
+github.com/godbus/dbus/v5 v5.1.0/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA=
 github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek=
 github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps=
 github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8=
@@ -85,6 +91,8 @@ github.com/google/pprof v0.0.0-20250317173921-a4b03ec1a45e h1:ijClszYn+mADRFY17k
 github.com/google/pprof v0.0.0-20250317173921-a4b03ec1a45e/go.mod h1:boTsfXsheKC2y+lKOCMpSfarhxDeIzfZG1jqGcPl3cA=
 github.com/google/s2a-go v0.1.9 h1:LGD7gtMgezd8a/Xak7mEWL0PjoTQFvpRudN895yqKW0=
 github.com/google/s2a-go v0.1.9/go.mod h1:YA0Ei2ZQL3acow2O62kdp9UlnvMmU7kA6Eutn0dXayM=
+github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 h1:El6M4kTTCOh6aBiKaUGG7oYTSPP8MxqL4YI3kZKwcP4=
+github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3pAXIBCelhxNneeOaAeabZDe5s4K6zSpQ=
 github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=
 github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/googleapis/enterprise-certificate-proxy v0.3.12 h1:Fg+zsqzYEs1ZnvmcztTYxhgCBsx3eEhEwQ1W/lHq/sQ=
@@ -173,6 +181,8 @@ github.com/spf13/cobra v1.9.1/go.mod h1:nDyEzZ8ogv936Cinf6g1RU9MRY64Ir93oCnqb9wx
 github.com/spf13/pflag v1.0.6 h1:jFzHGLGAlb3ruxLB8MhbI6A8+AQX/2eW4qeyNZXNp2o=
 github.com/spf13/pflag v1.0.6/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
+github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY=
+github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
 github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U=
@@ -192,6 +202,8 @@ github.com/vektah/gqlparser/v2 v2.5.27 h1:RHPD3JOplpk5mP5JGX8RKZkt2/Vwj/PZv0HxTd
 github.com/vektah/gqlparser/v2 v2.5.27/go.mod h1:D1/VCZtV3LPnQrcPBeR/q5jkSQIPti0uYCP/RI0gIeo=
 github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e h1:JVG44RsyaB9T2KIHavMF/ppJZNG9ZpyihvCd0w101no=
 github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e/go.mod h1:RbqR21r5mrJuqunuUZ/Dhy/avygyECGrLceyNeo4LiM=
+github.com/zalando/go-keyring v0.2.6 h1:r7Yc3+H+Ux0+M72zacZoItR3UDxeWfKTcabvkI8ua9s=
+github.com/zalando/go-keyring v0.2.6/go.mod h1:2TCrxYrbUNYfNS/Kgy/LSrkSQzZ5UPVH85RwfczwvcI=
 go.mau.fi/libsignal v0.2.1 h1:vRZG4EzTn70XY6Oh/pVKrQGuMHBkAWlGRC22/85m9L0=
 go.mau.fi/libsignal v0.2.1/go.mod h1:iVvjrHyfQqWajOUaMEsIfo3IqgVMrhWcPiiEzk7NgoU=
 go.mau.fi/util v0.9.6 h1:2nsvxm49KhI3wrFltr0+wSUBlnQ4CMtykuELjpIU+ts=

internal/cli/setup_models.go

@@ -272,7 +272,7 @@ func configureAPIKey(pcfg *config.ModelProviderConfig, name string, existing con
 	apiKey = cleanPath(apiKey)
 	if apiKey != "" {
 		ref := fmt.Sprintf("keychain:obk/%s", name)
-		if err := provider.KeychainStore(ref, apiKey); err != nil {
+		if err := provider.StoreCredential(ref, apiKey); err != nil {
 			return fmt.Errorf("store API key: %w", err)
 		}
 		pcfg.APIKeyRef = ref

provider/credential.go

@@ -3,34 +3,43 @@ package provider
 import (
 	"fmt"
 	"os"
-	"path/filepath"
 	"strings"
+
+	"github.com/zalando/go-keyring"
 )
 
-// KeychainLoad retrieves an API key from the platform credential store.
+// LoadCredential retrieves an API key from the OS keyring.
 // The ref format is "keychain:<service>/<account>", e.g. "keychain:obk/anthropic".
-func KeychainLoad(ref string) (string, error) {
+func LoadCredential(ref string) (string, error) {
 	service, account, err := parseCredentialRef(ref)
 	if err != nil {
 		return "", err
 	}
-	return credentialLoad(service, account)
+	val, err := keyring.Get(service, account)
+	if err != nil {
+		return "", fmt.Errorf("load credential %s/%s: %w", service, account, err)
+	}
+	return val, nil
 }
 
-// KeychainStore saves an API key to the platform credential store.
-func KeychainStore(ref, value string) error {
+// StoreCredential saves an API key to the OS keyring.
+func StoreCredential(ref, value string) error {
 	service, account, err := parseCredentialRef(ref)
 	if err != nil {
 		return err
 	}
-	return credentialStore(service, account, value)
+	if err := keyring.Set(service, account, value); err != nil {
+		return fmt.Errorf("store credential %s/%s: %w", service, account, err)
+	}
+	return nil
 }
 
-// ResolveAPIKey resolves an API key from either a credential store reference
-// or an environment variable fallback.
+// ResolveAPIKey resolves an API key from a keyring reference or
+// an environment variable. For headless/CI environments where no
+// keyring is available, set the environment variable directly.
 func ResolveAPIKey(ref, envVar string) (string, error) {
 	if ref != "" && strings.HasPrefix(ref, "keychain:") {
-		key, err := KeychainLoad(ref)
+		key, err := LoadCredential(ref)
 		if err == nil && key != "" {
 			return key, nil
 		}
@@ -53,51 +62,3 @@ func parseCredentialRef(ref string) (service, account string, err error) {
 	}
 	return parts[0], parts[1], nil
 }
-
-func secretsDir() (string, error) {
-	home, err := os.UserHomeDir()
-	if err != nil {
-		return "", fmt.Errorf("get home dir: %w", err)
-	}
-	return filepath.Join(home, ".obk", "secrets"), nil
-}
-
-func secretPath(service, account string) (string, error) {
-	dir, err := secretsDir()
-	if err != nil {
-		return "", err
-	}
-	return filepath.Join(dir, service+"-"+account), nil
-}
-
-// loadFromFile reads a credential from the file-based store.
-func loadFromFile(service, account string) (string, error) {
-	path, err := secretPath(service, account)
-	if err != nil {
-		return "", err
-	}
-	data, err := os.ReadFile(path)
-	if err != nil {
-		return "", fmt.Errorf("credential lookup failed for %s/%s: %w", service, account, err)
-	}
-	return strings.TrimSpace(string(data)), nil
-}
-
-// storeToFile writes a credential to the file-based store with 0600 permissions.
-func storeToFile(service, account, value string) error {
-	dir, err := secretsDir()
-	if err != nil {
-		return err
-	}
-	if err := os.MkdirAll(dir, 0700); err != nil {
-		return fmt.Errorf("create secrets dir: %w", err)
-	}
-	path, err := secretPath(service, account)
-	if err != nil {
-		return err
-	}
-	if err := os.WriteFile(path, []byte(value), 0600); err != nil {
-		return fmt.Errorf("store credential: %w", err)
-	}
-	return nil
-}