Skip to content

Commit 716f6c2

Browse files
steipetesteipete
committed
refactor(secrets): split keyring backend policy
1 parent d8af728 commit 716f6c2

2 files changed

Lines changed: 292 additions & 277 deletions

File tree

‎internal/secrets/backend.go‎

Lines changed: 288 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,288 @@
1+
package secrets
2+
3+
import (
4+
"errors"
5+
"fmt"
6+
"os"
7+
"reflect"
8+
"runtime"
9+
"strings"
10+
"time"
11+
12+
"github.com/99designs/keyring"
13+
14+
"github.com/steipete/gogcli/internal/config"
15+
"github.com/steipete/gogcli/internal/termutil"
16+
)
17+
18+
const (
19+
keyringPasswordEnv = "GOG_KEYRING_PASSWORD" //nolint:gosec // env var name, not a credential
20+
keyringBackendEnv = "GOG_KEYRING_BACKEND" //nolint:gosec // env var name, not a credential
21+
keyringServiceNameEnv = "GOG_KEYRING_SERVICE_NAME"
22+
)
23+
24+
var (
25+
errNoTTY = errors.New("no TTY available for keyring file backend password prompt")
26+
errInvalidKeyringBackend = errors.New("invalid keyring backend")
27+
errKeyringTimeout = errors.New("keyring connection timed out")
28+
openKeyringFunc = openKeyring
29+
keyringOpenFunc = keyring.Open
30+
)
31+
32+
type KeyringBackendInfo struct {
33+
Value string
34+
Source string
35+
}
36+
37+
const (
38+
keyringBackendSourceEnv = "env"
39+
keyringBackendSourceConfig = "config"
40+
keyringBackendSourceDefault = "default"
41+
keyringBackendAuto = "auto"
42+
)
43+
44+
func ResolveKeyringBackendInfo() (KeyringBackendInfo, error) {
45+
if v := normalizeKeyringBackend(os.Getenv(keyringBackendEnv)); v != "" {
46+
return KeyringBackendInfo{Value: v, Source: keyringBackendSourceEnv}, nil
47+
}
48+
49+
cfg, err := config.ReadConfig()
50+
if err != nil {
51+
return KeyringBackendInfo{}, fmt.Errorf("resolve keyring backend: %w", err)
52+
}
53+
54+
if cfg.KeyringBackend != "" {
55+
if v := normalizeKeyringBackend(cfg.KeyringBackend); v != "" {
56+
return KeyringBackendInfo{Value: v, Source: keyringBackendSourceConfig}, nil
57+
}
58+
}
59+
60+
return KeyringBackendInfo{Value: keyringBackendAuto, Source: keyringBackendSourceDefault}, nil
61+
}
62+
63+
func allowedBackends(info KeyringBackendInfo) ([]keyring.BackendType, error) {
64+
switch info.Value {
65+
case "", keyringBackendAuto:
66+
return nil, nil
67+
case "keychain":
68+
return []keyring.BackendType{keyring.KeychainBackend}, nil
69+
case "file":
70+
return []keyring.BackendType{keyring.FileBackend}, nil
71+
default:
72+
return nil, fmt.Errorf("%w: %q (expected %s, keychain, or file)", errInvalidKeyringBackend, info.Value, keyringBackendAuto)
73+
}
74+
}
75+
76+
// wrapKeychainError wraps keychain errors with helpful guidance on macOS.
77+
func wrapKeychainError(err error) error {
78+
if err == nil {
79+
return nil
80+
}
81+
82+
if IsKeychainLockedError(err.Error()) {
83+
return fmt.Errorf("%w\n\nYour macOS keychain is locked. To unlock it, run:\n security unlock-keychain ~/Library/Keychains/login.keychain-db", err)
84+
}
85+
86+
return err
87+
}
88+
89+
func fileKeyringPasswordFuncFrom(password string, passwordSet bool, isTTY bool) keyring.PromptFunc {
90+
// Treat "set to empty string" as intentional; empty passphrase is valid.
91+
if passwordSet {
92+
return keyring.FixedStringPrompt(password)
93+
}
94+
95+
if isTTY {
96+
return keyring.TerminalPrompt
97+
}
98+
99+
return func(_ string) (string, error) {
100+
return "", fmt.Errorf("%w; set %s", errNoTTY, keyringPasswordEnv)
101+
}
102+
}
103+
104+
func fileKeyringPasswordFunc() keyring.PromptFunc {
105+
password, passwordSet := os.LookupEnv(keyringPasswordEnv)
106+
return fileKeyringPasswordFuncFrom(password, passwordSet, termutil.IsTerminal(os.Stdin))
107+
}
108+
109+
func normalizeKeyringBackend(value string) string {
110+
return strings.ToLower(strings.TrimSpace(value))
111+
}
112+
113+
func keyringServiceName() string {
114+
if serviceName := strings.TrimSpace(os.Getenv(keyringServiceNameEnv)); serviceName != "" {
115+
return serviceName
116+
}
117+
118+
return config.AppName
119+
}
120+
121+
// keyringOpenTimeout is the maximum time to wait for keyring.Open() to complete.
122+
// On headless Linux, D-Bus SecretService can hang indefinitely if gnome-keyring
123+
// is installed but not running.
124+
const (
125+
keyringOpenTimeout = 10 * time.Second
126+
goosDarwin = "darwin"
127+
goosLinux = "linux"
128+
)
129+
130+
func shouldForceFileBackend(goos string, backendInfo KeyringBackendInfo, dbusAddr string) bool {
131+
return goos == goosLinux && backendInfo.Value == keyringBackendAuto && dbusAddr == ""
132+
}
133+
134+
func shouldUseKeyringTimeout(goos string, backendInfo KeyringBackendInfo, dbusAddr string) bool {
135+
return goos == goosLinux && backendInfo.Value == keyringBackendAuto && dbusAddr != ""
136+
}
137+
138+
func shouldUseKeyringOperationTimeout(goos string, backendInfo KeyringBackendInfo, dbusAddr string) bool {
139+
if goos == goosDarwin {
140+
return backendInfo.Value == keyringBackendAuto || backendInfo.Value == "keychain"
141+
}
142+
143+
return goos == goosLinux && backendInfo.Value == keyringBackendAuto && dbusAddr != ""
144+
}
145+
146+
func keyringTimeoutHint(goos string) string {
147+
switch goos {
148+
case goosDarwin:
149+
return "macOS Keychain may be waiting for a permission prompt; run `gog auth list` from a terminal and click \"Always Allow\" when prompted"
150+
case goosLinux:
151+
return "D-Bus SecretService may be unresponsive"
152+
default:
153+
return "keyring backend may be unresponsive"
154+
}
155+
}
156+
157+
func isFileKeyring(ring keyring.Keyring) bool {
158+
if ring == nil {
159+
return false
160+
}
161+
162+
return reflect.TypeOf(ring).String() == "*keyring.fileKeyring"
163+
}
164+
165+
func openKeyring() (keyring.Keyring, error) {
166+
// On Linux/WSL/containers, OS keychains (secret-service/kwallet) may be unavailable.
167+
// In that case github.com/99designs/keyring falls back to the "file" backend,
168+
// which *requires* both a directory and a password prompt function.
169+
keyringDir, err := config.EnsureKeyringDir()
170+
if err != nil {
171+
return nil, fmt.Errorf("ensure keyring dir: %w", err)
172+
}
173+
174+
backendInfo, err := ResolveKeyringBackendInfo()
175+
if err != nil {
176+
return nil, err
177+
}
178+
179+
backends, err := allowedBackends(backendInfo)
180+
if err != nil {
181+
return nil, err
182+
}
183+
wrapFileKeys := fileKeyringBackendOnly(backends)
184+
185+
dbusAddr := os.Getenv("DBUS_SESSION_BUS_ADDRESS")
186+
// On Linux with "auto" backend and no D-Bus session, force file backend.
187+
// Without DBUS_SESSION_BUS_ADDRESS, SecretService will hang indefinitely
188+
// trying to connect (common on headless systems like Raspberry Pi).
189+
if shouldForceFileBackend(runtime.GOOS, backendInfo, dbusAddr) {
190+
backends = []keyring.BackendType{keyring.FileBackend}
191+
wrapFileKeys = true
192+
}
193+
194+
cfg := keyring.Config{
195+
ServiceName: keyringServiceName(),
196+
// KeychainTrustApplication is intentionally false to support Homebrew upgrades.
197+
// When true, macOS Keychain ties access control to the specific binary hash.
198+
// Homebrew upgrades install a new binary with a different hash, causing the
199+
// new binary to lose access to existing keychain items. With false, users may
200+
// see a one-time keychain prompt after upgrade (click "Always Allow"), but
201+
// tokens survive across upgrades. See: https://github.com/steipete/gogcli/issues/86
202+
KeychainTrustApplication: false,
203+
AllowedBackends: backends,
204+
FileDir: keyringDir,
205+
FilePasswordFunc: fileKeyringPasswordFunc(),
206+
}
207+
208+
// On Linux with D-Bus present, keyring.Open() can still hang if SecretService
209+
// is unresponsive (e.g., gnome-keyring installed but not running).
210+
// Use a timeout as a safety net.
211+
if shouldUseKeyringTimeout(runtime.GOOS, backendInfo, dbusAddr) {
212+
timeoutRing, timeoutErr := openKeyringWithTimeout(cfg, keyringOpenTimeout)
213+
if timeoutErr != nil {
214+
return nil, timeoutErr
215+
}
216+
217+
return prepareKeyring(timeoutRing, backendInfo, wrapFileKeys, dbusAddr), nil
218+
}
219+
220+
ring, err := keyringOpenFunc(cfg)
221+
if err != nil {
222+
return nil, fmt.Errorf("open keyring: %w", err)
223+
}
224+
225+
return prepareKeyring(ring, backendInfo, wrapFileKeys, dbusAddr), nil
226+
}
227+
228+
func prepareKeyring(ring keyring.Keyring, backendInfo KeyringBackendInfo, wrapFileKeys bool, dbusAddr string) keyring.Keyring {
229+
if wrapFileKeys || isFileKeyring(ring) {
230+
ring = newFileSafeKeyring(ring)
231+
}
232+
233+
if shouldUseKeyringOperationTimeout(runtime.GOOS, backendInfo, dbusAddr) {
234+
ring = newTimeoutKeyring(ring, keyringOpenTimeout, keyringTimeoutHint(runtime.GOOS))
235+
}
236+
237+
return ring
238+
}
239+
240+
type keyringResult struct {
241+
ring keyring.Keyring
242+
err error
243+
}
244+
245+
// openKeyringWithTimeout wraps keyring.Open with a timeout to prevent indefinite
246+
// hangs when D-Bus SecretService is unresponsive (e.g., gnome-keyring installed
247+
// but not running on headless Linux).
248+
//
249+
// Note: If timeout occurs, the spawned goroutine continues blocking on keyring.Open()
250+
// and will leak. This is acceptable for a CLI tool since the process exits on this
251+
// error, but would need refactoring for long-running use.
252+
func openKeyringWithTimeout(cfg keyring.Config, timeout time.Duration) (keyring.Keyring, error) {
253+
return openKeyringWithTimeoutFunc(cfg, timeout, keyringOpenFunc)
254+
}
255+
256+
func openKeyringWithTimeoutFunc(cfg keyring.Config, timeout time.Duration, open func(keyring.Config) (keyring.Keyring, error)) (keyring.Keyring, error) {
257+
ch := make(chan keyringResult, 1)
258+
259+
go func() {
260+
ring, err := open(cfg)
261+
ch <- keyringResult{ring, err}
262+
}()
263+
264+
select {
265+
case res := <-ch:
266+
if res.err != nil {
267+
return nil, fmt.Errorf("open keyring: %w", res.err)
268+
}
269+
270+
return res.ring, nil
271+
case <-time.After(timeout):
272+
return nil, keyringTimeoutError("opening keyring", timeout, keyringTimeoutHint(runtime.GOOS))
273+
}
274+
}
275+
276+
func OpenDefault() (Store, error) {
277+
ring, err := openKeyringFunc()
278+
if err != nil {
279+
return nil, err
280+
}
281+
282+
lock, _, err := keyringLockForRing(ring)
283+
if err != nil {
284+
return nil, err
285+
}
286+
287+
return &KeyringStore{ring: ring, lock: lock}, nil
288+
}

0 commit comments

Comments
 (0)