Make keyring availability checks read-only

internal/auth/store.go

@@ -2,17 +2,38 @@ package auth
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
 	"runtime"
 	"sync"
 
-	"github.com/zalando/go-keyring"
+	keyringlib "github.com/zalando/go-keyring"
 )
 
 const serviceName = "hey"
 
+type credentialKeyring interface {
+	Get(service, user string) (string, error)
+	Set(service, user, password string) error
+	Delete(service, user string) error
+}
+
+type systemCredentialKeyring struct{}
+
+func (systemCredentialKeyring) Get(service, user string) (string, error) {
+	return keyringlib.Get(service, user)
+}
+
+func (systemCredentialKeyring) Set(service, user, password string) error {
+	return keyringlib.Set(service, user, password)
+}
+
+func (systemCredentialKeyring) Delete(service, user string) error {
+	return keyringlib.Delete(service, user)
+}
+
 // Credentials holds OAuth tokens and metadata.
 type Credentials struct {
 	AccessToken   string `json:"access_token"`  //nolint:gosec // G117: legitimate credential field
@@ -29,6 +50,7 @@ type Store struct {
 	useKeyring  bool
 	noKeyring   bool
 	fallbackDir string
+	keyring     credentialKeyring
 }
 
 // NewStore creates a credential store. Keyring availability is probed lazily
@@ -37,6 +59,7 @@ func NewStore(fallbackDir string) *Store {
 	return &Store{
 		fallbackDir: fallbackDir,
 		noKeyring:   os.Getenv("HEY_NO_KEYRING") != "",
+		keyring:     systemCredentialKeyring{},
 	}
 }
 
@@ -45,10 +68,8 @@ func (s *Store) ensureInit() {
 		if s.noKeyring {
 			return
 		}
-		testKey := "hey::test"
-		err := keyring.Set(serviceName, testKey, "test")
-		if err == nil {
-			_ = keyring.Delete(serviceName, testKey)
+		_, err := s.keyring.Get(serviceName, "hey::availability")
+		if err == nil || errors.Is(err, keyringlib.ErrNotFound) {
 			s.useKeyring = true
 			return
 		}
@@ -83,13 +104,13 @@ func (s *Store) Save(origin string, creds *Credentials) error {
 func (s *Store) Delete(origin string) error {
 	s.ensureInit()
 	if s.useKeyring {
-		return keyring.Delete(serviceName, key(origin))
+		return s.keyring.Delete(serviceName, key(origin))
 	}
 	return s.deleteFile(origin)
 }
 
 func (s *Store) loadFromKeyring(origin string) (*Credentials, error) {
-	data, err := keyring.Get(serviceName, key(origin))
+	data, err := s.keyring.Get(serviceName, key(origin))
 	if err != nil {
 		return nil, fmt.Errorf("credentials not found: %w", err)
 	}
@@ -106,7 +127,7 @@ func (s *Store) saveToKeyring(origin string, creds *Credentials) error {
 	if err != nil {
 		return err
 	}
-	return keyring.Set(serviceName, key(origin), string(data))
+	return s.keyring.Set(serviceName, key(origin), string(data))
 }
 
 func (s *Store) credentialsPath() string {

internal/auth/store_test.go

@@ -1,11 +1,36 @@
 package auth
 
 import (
+	"errors"
 	"os"
 	"path/filepath"
 	"testing"
+
+	keyringlib "github.com/zalando/go-keyring"
 )
 
+type recordingKeyring struct {
+	getCalls    int
+	setCalls    int
+	deleteCalls int
+	getErr      error
+}
+
+func (k *recordingKeyring) Get(_, _ string) (string, error) {
+	k.getCalls++
+	return "", k.getErr
+}
+
+func (k *recordingKeyring) Set(_, _, _ string) error {
+	k.setCalls++
+	return nil
+}
+
+func (k *recordingKeyring) Delete(_, _ string) error {
+	k.deleteCalls++
+	return nil
+}
+
 func testStore(t *testing.T) *Store {
 	t.Helper()
 	t.Setenv("HEY_NO_KEYRING", "1")
@@ -53,6 +78,34 @@ func TestLoadNotFound(t *testing.T) {
 	}
 }
 
+func TestKeyringAvailabilityCheckIsReadOnly(t *testing.T) {
+	t.Setenv("HEY_NO_KEYRING", "")
+	keyring := &recordingKeyring{getErr: keyringlib.ErrNotFound}
+	store := NewStore(t.TempDir())
+	store.keyring = keyring
+
+	if !store.UsingKeyring() {
+		t.Fatal("expected missing availability key to confirm keyring access")
+	}
+	if keyring.getCalls != 1 {
+		t.Errorf("Get calls = %d, want 1", keyring.getCalls)
+	}
+	if keyring.setCalls != 0 || keyring.deleteCalls != 0 {
+		t.Errorf("availability check mutated keyring: Set = %d, Delete = %d", keyring.setCalls, keyring.deleteCalls)
+	}
+}
+
+func TestKeyringAvailabilityErrorUsesFileStore(t *testing.T) {
+	t.Setenv("HEY_NO_KEYRING", "")
+	keyring := &recordingKeyring{getErr: errors.New("keyring unavailable")}
+	store := NewStore(t.TempDir())
+	store.keyring = keyring
+
+	if store.UsingKeyring() {
+		t.Fatal("expected unavailable keyring to use file store")
+	}
+}
+
 func TestDeleteCredentials(t *testing.T) {
 	s := testStore(t)
 	origin := "https://app.hey.com"