fix(secrets): time out macOS keyring operations

Co-authored-by: Sardor Umarov <sardoru@gmail.com>

CHANGELOG.md

@@ -7,6 +7,7 @@
 - Backup: expand `gog backup push --services all` with Drive content export/download, Gmail settings, native Workspace Docs/Sheets/Slides/Form data, Apps Script projects, Chat, Classroom, best-effort optional service error shards, and plaintext Drive file export.
 
 ### Fixed
+- Secrets: time out macOS Keychain read/write/list operations with a clear recovery hint instead of hanging indefinitely when a permission prompt cannot surface. (#515, #513) — thanks @sardoru.
 - Secrets: encode file-backend key names so stored tokens work on Windows, while still reading/removing legacy raw entries. (#527, #502) — thanks @solomonneas.
 - Drive: include `driveId` in `drive ls`, `drive search`, and `drive get` field masks so Shared Drive files can be identified in JSON output. (#524) — thanks @LeanSheng.
 - Gmail: expose reply threading headers in default `gmail get --format metadata` output and fail explicit reply targets that cannot provide a `Message-ID`. (#528, #512) — thanks @solomonneas.

README.md

@@ -258,6 +258,8 @@ Backends:
 - `keychain`: macOS Keychain (recommended on macOS; avoids password management).
 - `file`: encrypted on-disk keyring (requires a password).
 
+On macOS, Keychain operations time out with a recovery hint if a permission prompt cannot surface; run the command from Terminal and choose "Always Allow", or switch to the file backend for headless runs.
+
 Set backend via command (writes `keyring_backend` into `config.json`):
 
 ```bash

docs/spec.md

@@ -93,6 +93,7 @@ Implementation: `internal/config/*`.
 - Key format: `token:<client>:<email>` (default client uses `token:default:<email>`)
 - Legacy key format: `token:<email>` (migrated on first read)
 - Stored payload is JSON (refresh token + metadata like selected services/scopes).
+- macOS Keychain operations are bounded by a timeout so non-surfacing permission prompts return actionable guidance instead of hanging indefinitely.
 - Fallback: if no OS credential store is available, keyring may use its encrypted "file" backend:
   - Directory: `$(os.UserConfigDir())/gogcli/keyring/` (one file per key; gog-managed key names are encoded for portable filenames)
   - Password: prompts on TTY; for non-interactive runs set `GOG_KEYRING_PASSWORD`

internal/secrets/store.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"os"
+	"reflect"
 	"runtime"
 	"strings"
 	"time"
@@ -156,7 +157,7 @@ func keyringServiceName() string {
 // keyringOpenTimeout is the maximum time to wait for keyring.Open() to complete.
 // On headless Linux, D-Bus SecretService can hang indefinitely if gnome-keyring
 // is installed but not running.
-const keyringOpenTimeout = 5 * time.Second
+const keyringOpenTimeout = 10 * time.Second
 
 func shouldForceFileBackend(goos string, backendInfo KeyringBackendInfo, dbusAddr string) bool {
 	return goos == "linux" && backendInfo.Value == keyringBackendAuto && dbusAddr == ""
@@ -166,6 +167,29 @@ func shouldUseKeyringTimeout(goos string, backendInfo KeyringBackendInfo, dbusAd
 	return goos == "linux" && backendInfo.Value == "auto" && dbusAddr != ""
 }
 
+func shouldUseKeyringOperationTimeout(goos string, backendInfo KeyringBackendInfo) bool {
+	return goos == "darwin" && (backendInfo.Value == keyringBackendAuto || backendInfo.Value == "keychain")
+}
+
+func keyringTimeoutHint(goos string) string {
+	switch goos {
+	case "darwin":
+		return "macOS Keychain may be waiting for a permission prompt; run `gog auth list` from a terminal and click \"Always Allow\" when prompted"
+	case "linux":
+		return "D-Bus SecretService may be unresponsive"
+	default:
+		return "keyring backend may be unresponsive"
+	}
+}
+
+func isFileKeyring(ring keyring.Keyring) bool {
+	if ring == nil {
+		return false
+	}
+
+	return reflect.TypeOf(ring).String() == "*keyring.fileKeyring"
+}
+
 func openKeyring() (keyring.Keyring, error) {
 	// On Linux/WSL/containers, OS keychains (secret-service/kwallet) may be unavailable.
 	// In that case github.com/99designs/keyring falls back to the "file" backend,
@@ -217,10 +241,7 @@ func openKeyring() (keyring.Keyring, error) {
 		if err != nil {
 			return nil, err
 		}
-		if wrapFileKeys {
-			return newFileSafeKeyring(ring), nil
-		}
-		return ring, nil
+		return prepareKeyring(ring, backendInfo, wrapFileKeys), nil
 	}
 
 	ring, err := keyringOpenFunc(cfg)
@@ -228,11 +249,18 @@ func openKeyring() (keyring.Keyring, error) {
 		return nil, fmt.Errorf("open keyring: %w", err)
 	}
 
-	if wrapFileKeys {
-		return newFileSafeKeyring(ring), nil
+	return prepareKeyring(ring, backendInfo, wrapFileKeys), nil
+}
+
+func prepareKeyring(ring keyring.Keyring, backendInfo KeyringBackendInfo, wrapFileKeys bool) keyring.Keyring {
+	if wrapFileKeys || isFileKeyring(ring) {
+		ring = newFileSafeKeyring(ring)
+	}
+	if shouldUseKeyringOperationTimeout(runtime.GOOS, backendInfo) {
+		ring = newTimeoutKeyring(ring, keyringOpenTimeout, keyringTimeoutHint(runtime.GOOS))
 	}
 
-	return ring, nil
+	return ring
 }
 
 type keyringResult struct {
@@ -263,9 +291,7 @@ func openKeyringWithTimeout(cfg keyring.Config, timeout time.Duration) (keyring.
 
 		return res.ring, nil
 	case <-time.After(timeout):
-		return nil, fmt.Errorf("%w after %v (D-Bus SecretService may be unresponsive); "+
-			"set GOG_KEYRING_BACKEND=file and GOG_KEYRING_PASSWORD=<password> to use encrypted file storage instead",
-			errKeyringTimeout, timeout)
+		return nil, keyringTimeoutError("opening keyring", timeout, keyringTimeoutHint(runtime.GOOS))
 	}
 }
 

internal/secrets/store_test.go

@@ -172,6 +172,14 @@ func TestKeyringDbusGuards(t *testing.T) {
 			wantForce:   false,
 			wantTimeout: false,
 		},
+		{
+			name:        "darwin auto no open timeout",
+			goos:        "darwin",
+			backend:     "auto",
+			dbusAddr:    "",
+			wantForce:   false,
+			wantTimeout: false,
+		},
 		{
 			name:        "linux explicit file no dbus",
 			goos:        "linux",

internal/secrets/timeout_keyring.go

@@ -0,0 +1,79 @@
+package secrets
+
+import (
+	"fmt"
+	"time"
+
+	"github.com/99designs/keyring"
+)
+
+type timeoutKeyring struct {
+	inner   keyring.Keyring
+	timeout time.Duration
+	hint    string
+}
+
+func newTimeoutKeyring(inner keyring.Keyring, timeout time.Duration, hint string) keyring.Keyring {
+	return &timeoutKeyring{
+		inner:   inner,
+		timeout: timeout,
+		hint:    hint,
+	}
+}
+
+func withKeyringTimeout[T any](timeout time.Duration, operation string, hint string, fn func() (T, error)) (T, error) {
+	type result struct {
+		value T
+		err   error
+	}
+
+	ch := make(chan result, 1)
+	go func() {
+		value, err := fn()
+		ch <- result{value: value, err: err}
+	}()
+
+	select {
+	case res := <-ch:
+		return res.value, res.err
+	case <-time.After(timeout):
+		var zero T
+		return zero, keyringTimeoutError(operation, timeout, hint)
+	}
+}
+
+func keyringTimeoutError(operation string, timeout time.Duration, hint string) error {
+	return fmt.Errorf("%w after %v while %s (%s); "+
+		"set GOG_KEYRING_BACKEND=file and GOG_KEYRING_PASSWORD=<password> to use encrypted file storage instead",
+		errKeyringTimeout, timeout, operation, hint)
+}
+
+func (k *timeoutKeyring) Get(key string) (keyring.Item, error) {
+	return withKeyringTimeout(k.timeout, "reading keyring item", k.hint, func() (keyring.Item, error) {
+		return k.inner.Get(key)
+	})
+}
+
+func (k *timeoutKeyring) GetMetadata(key string) (keyring.Metadata, error) {
+	return withKeyringTimeout(k.timeout, "reading keyring metadata", k.hint, func() (keyring.Metadata, error) {
+		return k.inner.GetMetadata(key)
+	})
+}
+
+func (k *timeoutKeyring) Set(item keyring.Item) error {
+	_, err := withKeyringTimeout(k.timeout, "storing keyring item", k.hint, func() (struct{}, error) {
+		return struct{}{}, k.inner.Set(item)
+	})
+	return err
+}
+
+func (k *timeoutKeyring) Remove(key string) error {
+	_, err := withKeyringTimeout(k.timeout, "removing keyring item", k.hint, func() (struct{}, error) {
+		return struct{}{}, k.inner.Remove(key)
+	})
+	return err
+}
+
+func (k *timeoutKeyring) Keys() ([]string, error) {
+	return withKeyringTimeout(k.timeout, "listing keyring items", k.hint, k.inner.Keys)
+}

internal/secrets/timeout_keyring_test.go

@@ -0,0 +1,102 @@
+package secrets
+
+import (
+	"errors"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/99designs/keyring"
+)
+
+func TestKeyringOperationTimeoutGuards(t *testing.T) {
+	tests := []struct {
+		name     string
+		goos     string
+		backend  string
+		wantWrap bool
+	}{
+		{name: "darwin auto", goos: "darwin", backend: "auto", wantWrap: true},
+		{name: "darwin keychain", goos: "darwin", backend: "keychain", wantWrap: true},
+		{name: "darwin file", goos: "darwin", backend: "file", wantWrap: false},
+		{name: "linux auto", goos: "linux", backend: "auto", wantWrap: false},
+		{name: "windows auto", goos: "windows", backend: "auto", wantWrap: false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := shouldUseKeyringOperationTimeout(tt.goos, KeyringBackendInfo{Value: tt.backend})
+			if got != tt.wantWrap {
+				t.Fatalf("shouldUseKeyringOperationTimeout=%v, want %v", got, tt.wantWrap)
+			}
+		})
+	}
+}
+
+func TestKeyringTimeoutHint(t *testing.T) {
+	tests := []struct {
+		goos       string
+		wantSubstr string
+	}{
+		{"darwin", "Always Allow"},
+		{"linux", "D-Bus SecretService"},
+		{"windows", "keyring backend"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.goos, func(t *testing.T) {
+			hint := keyringTimeoutHint(tt.goos)
+			if !strings.Contains(hint, tt.wantSubstr) {
+				t.Fatalf("keyringTimeoutHint(%q)=%q, want substring %q", tt.goos, hint, tt.wantSubstr)
+			}
+		})
+	}
+}
+
+func TestTimeoutKeyringTimesOutOperations(t *testing.T) {
+	block := make(chan struct{})
+	t.Cleanup(func() { close(block) })
+
+	ring := newTimeoutKeyring(&blockingKeyring{block: block}, 10*time.Millisecond, keyringTimeoutHint("darwin"))
+
+	_, err := ring.Keys()
+	if !errors.Is(err, errKeyringTimeout) {
+		t.Fatalf("expected timeout error, got %v", err)
+	}
+	if !strings.Contains(err.Error(), "listing keyring items") || !strings.Contains(err.Error(), "Always Allow") {
+		t.Fatalf("expected operation and macOS hint in timeout, got %v", err)
+	}
+}
+
+type blockingKeyring struct {
+	block <-chan struct{}
+}
+
+func (k *blockingKeyring) wait() {
+	<-k.block
+}
+
+func (k *blockingKeyring) Get(string) (keyring.Item, error) {
+	k.wait()
+	return keyring.Item{}, keyring.ErrKeyNotFound
+}
+
+func (k *blockingKeyring) GetMetadata(string) (keyring.Metadata, error) {
+	k.wait()
+	return keyring.Metadata{}, keyring.ErrKeyNotFound
+}
+
+func (k *blockingKeyring) Set(keyring.Item) error {
+	k.wait()
+	return nil
+}
+
+func (k *blockingKeyring) Remove(string) error {
+	k.wait()
+	return nil
+}
+
+func (k *blockingKeyring) Keys() ([]string, error) {
+	k.wait()
+	return nil, nil
+}