openclaw/gogcli
openclaw/gogcli
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 3 Wiki Activity

test(secrets): satisfy keyring lint rules

Browse Source
This commit is contained in:
Peter Steinberger 2026-04-27 11:35:08 +01:00
parent 46331eabc3
commit c2ea4f55e0
No known key found for this signature in database
5 changed files with 67 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
internal/secrets/file_keyring_safe.go
View File

@ -3,6 +3,7 @@ package secrets
import (
"encoding/base64"
"errors"
"fmt"
"io/fs"
"sort"
"strings"
@ -61,8 +62,9 @@ func (k *fileSafeKeyring) Get(key string) (keyring.Item, error) {
item.Key = key
return item, nil
}
if !fileKeyNotFound(err) {
return keyring.Item{}, err
return keyring.Item{}, fmt.Errorf("read encoded file keyring item: %w", err)
}
item, legacyErr := k.inner.Get(key)
@ -70,10 +72,12 @@ func (k *fileSafeKeyring) Get(key string) (keyring.Item, error) {
if fileKeyNotFound(legacyErr) {
return keyring.Item{}, keyring.ErrKeyNotFound
}
return keyring.Item{}, legacyErr
return keyring.Item{}, fmt.Errorf("read legacy file keyring item: %w", legacyErr)
}
item.Key = key
return item, nil
}
@ -82,8 +86,9 @@ func (k *fileSafeKeyring) GetMetadata(key string) (keyring.Metadata, error) {
if err == nil {
return meta, nil
}
if !fileKeyNotFound(err) {
return keyring.Metadata{}, err
return keyring.Metadata{}, fmt.Errorf("read encoded file keyring metadata: %w", err)
}
meta, legacyErr := k.inner.GetMetadata(key)
@ -91,7 +96,8 @@ func (k *fileSafeKeyring) GetMetadata(key string) (keyring.Metadata, error) {
if fileKeyNotFound(legacyErr) {
return keyring.Metadata{}, keyring.ErrKeyNotFound
}
return keyring.Metadata{}, legacyErr
return keyring.Metadata{}, fmt.Errorf("read legacy file keyring metadata: %w", legacyErr)
}
return meta, nil
@ -99,7 +105,11 @@ func (k *fileSafeKeyring) GetMetadata(key string) (keyring.Metadata, error) {
func (k *fileSafeKeyring) Set(item keyring.Item) error {
item.Key = fileSafeKey(item.Key)
return k.inner.Set(item)
if err := k.inner.Set(item); err != nil {
return fmt.Errorf("store file keyring item: %w", err)
}
return nil
}
func (k *fileSafeKeyring) Remove(key string) error {
@ -109,22 +119,26 @@ func (k *fileSafeKeyring) Remove(key string) error {
if encodedErr == nil || legacyErr == nil {
return nil
}
if fileKeyNotFound(encodedErr) && fileKeyNotFound(legacyErr) {
return keyring.ErrKeyNotFound
}
if !fileKeyNotFound(encodedErr) {
return encodedErr
return fmt.Errorf("remove encoded file keyring item: %w", encodedErr)
}
return legacyErr
return fmt.Errorf("remove legacy file keyring item: %w", legacyErr)
}
func (k *fileSafeKeyring) Keys() ([]string, error) {
keys, err := k.inner.Keys()
if err != nil {
return nil, err
return nil, fmt.Errorf("list file keyring keys: %w", err)
}
out := make([]string, 0, len(keys))
seen := make(map[string]struct{}, len(keys))
for _, key := range keys {
decoded := decodeFileSafeKey(key)
@ -134,6 +148,7 @@ func (k *fileSafeKeyring) Keys() ([]string, error) {
seen[decoded] = struct{}{}
out = append(out, decoded)
}
sort.Strings(out)
return out, nil

27
internal/secrets/file_keyring_safe_test.go
View File

@ -27,9 +27,11 @@ func TestFileSafeKeyRoundTrip(t *testing.T) {
if encoded == key {
t.Fatalf("expected encoded key for %q", key)
}
if strings.ContainsAny(encoded, `<>:"/\|?*`) {
t.Fatalf("encoded key %q still contains a Windows filename separator/reserved char", encoded)
}
if got := decodeFileSafeKey(encoded); got != key {
t.Fatalf("decodeFileSafeKey(%q)=%q, want %q", encoded, got, key)
}
@ -39,6 +41,7 @@ func TestFileSafeKeyRoundTrip(t *testing.T) {
if got := decodeFileSafeKey(rawPrefixKey); got != "test" {
t.Fatalf("expected canonical encoded key to decode, got %q", got)
}
if got := decodeFileSafeKey(fileKeyPrefix + "not valid"); got != fileKeyPrefix+"not valid" {
t.Fatalf("expected invalid encoded key to remain raw, got %q", got)
}
@ -46,6 +49,7 @@ func TestFileSafeKeyRoundTrip(t *testing.T) {
func TestFileSafeKeyringRoundTripWithFileBackend(t *testing.T) {
dir := t.TempDir()
inner, err := keyring.Open(keyring.Config{
ServiceName: keyringServiceName(),
AllowedBackends: []keyring.BackendType{keyring.FileBackend},
@ -58,14 +62,16 @@ func TestFileSafeKeyringRoundTripWithFileBackend(t *testing.T) {
ring := newFileSafeKeyring(inner)
key := "token:default:user@example.com"
if err := ring.Set(keyring.Item{Key: key, Data: []byte("secret")}); err != nil {
t.Fatalf("Set: %v", err)
if setErr := ring.Set(keyring.Item{Key: key, Data: []byte("secret")}); setErr != nil {
t.Fatalf("Set: %v", setErr)
}
item, err := ring.Get(key)
if err != nil {
t.Fatalf("Get: %v", err)
}
if item.Key != key || string(item.Data) != "secret" {
t.Fatalf("unexpected item: key=%q data=%q", item.Key, item.Data)
}
@ -74,6 +80,7 @@ func TestFileSafeKeyringRoundTripWithFileBackend(t *testing.T) {
if err != nil {
t.Fatalf("Keys: %v", err)
}
if !slices.Contains(keys, key) {
t.Fatalf("expected decoded key %q in %v", key, keys)
}
@ -82,9 +89,11 @@ func TestFileSafeKeyringRoundTripWithFileBackend(t *testing.T) {
if err != nil {
t.Fatalf("ReadDir: %v", err)
}
if len(entries) != 1 {
t.Fatalf("expected one keyring file, got %d", len(entries))
}
if name := entries[0].Name(); strings.ContainsAny(name, `<>:"/\|?*`) {
t.Fatalf("keyring filename %q contains a Windows filename separator/reserved char", name)
}
@ -92,6 +101,7 @@ func TestFileSafeKeyringRoundTripWithFileBackend(t *testing.T) {
if err := ring.Remove(key); err != nil {
t.Fatalf("Remove: %v", err)
}
if _, err := ring.Get(key); !errors.Is(err, keyring.ErrKeyNotFound) {
t.Fatalf("expected not found after remove, got %v", err)
}
@ -110,24 +120,28 @@ func TestFileSafeKeyringReadsAndRemovesLegacyRawKeys(t *testing.T) {
if err != nil {
t.Fatalf("Get legacy key: %v", err)
}
if item.Key != key || string(item.Data) != "legacy" {
t.Fatalf("unexpected legacy item: key=%q data=%q", item.Key, item.Data)
}
if err := ring.Set(keyring.Item{Key: key, Data: []byte("new")}); err != nil {
t.Fatalf("Set encoded key: %v", err)
if setErr := ring.Set(keyring.Item{Key: key, Data: []byte("new")}); setErr != nil {
t.Fatalf("Set encoded key: %v", setErr)
}
keys, err := ring.Keys()
if err != nil {
t.Fatalf("Keys: %v", err)
}
got := 0
for _, listedKey := range keys {
if listedKey == key {
got++
}
}
if got != 1 {
t.Fatalf("expected one decoded key in %v, got count %d", keys, got)
}
@ -135,9 +149,11 @@ func TestFileSafeKeyringReadsAndRemovesLegacyRawKeys(t *testing.T) {
if err := ring.Remove(key); err != nil {
t.Fatalf("Remove: %v", err)
}
if _, err := inner.Get(key); !errors.Is(err, keyring.ErrKeyNotFound) {
t.Fatalf("expected legacy key removed, got %v", err)
}
if _, err := inner.Get(fileSafeKey(key)); !errors.Is(err, keyring.ErrKeyNotFound) {
t.Fatalf("expected encoded key removed, got %v", err)
}
@ -146,6 +162,7 @@ func TestFileSafeKeyringReadsAndRemovesLegacyRawKeys(t *testing.T) {
func TestFileSafeKeyringTreatsInvalidLegacyFilenameAsNotFound(t *testing.T) {
orig := isInvalidFileKeyError
isInvalidFileKeyError = func(err error) bool { return errors.Is(err, errInvalidTestFilename) }
t.Cleanup(func() { isInvalidFileKeyError = orig })
ring := newFileSafeKeyring(&invalidFilenameKeyring{})
@ -165,6 +182,7 @@ func TestOpenKeyringWrapsExplicitFileBackend(t *testing.T) {
keyringOpenFunc = func(_ keyring.Config) (keyring.Keyring, error) {
return keyring.NewArrayKeyring(nil), nil
}
t.Cleanup(func() { keyringOpenFunc = origOpen })
store, err := OpenDefault()
@ -176,6 +194,7 @@ func TestOpenKeyringWrapsExplicitFileBackend(t *testing.T) {
if !ok {
t.Fatalf("expected *KeyringStore, got %T", store)
}
if _, ok := keyringStore.ring.(*fileSafeKeyring); !ok {
t.Fatalf("expected file-safe keyring, got %T", keyringStore.ring)
}

26
internal/secrets/store.go
View File

@ -157,25 +157,29 @@ 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 = 10 * time.Second
const (
keyringOpenTimeout = 10 * time.Second
goosDarwin = "darwin"
goosLinux = "linux"
)
func shouldForceFileBackend(goos string, backendInfo KeyringBackendInfo, dbusAddr string) bool {
return goos == "linux" && backendInfo.Value == keyringBackendAuto && dbusAddr == ""
return goos == goosLinux && backendInfo.Value == keyringBackendAuto && dbusAddr == ""
}
func shouldUseKeyringTimeout(goos string, backendInfo KeyringBackendInfo, dbusAddr string) bool {
return goos == "linux" && backendInfo.Value == "auto" && dbusAddr != ""
return goos == goosLinux && backendInfo.Value == "auto" && dbusAddr != ""
}
func shouldUseKeyringOperationTimeout(goos string, backendInfo KeyringBackendInfo) bool {
return goos == "darwin" && (backendInfo.Value == keyringBackendAuto || backendInfo.Value == "keychain")
return goos == goosDarwin && (backendInfo.Value == keyringBackendAuto || backendInfo.Value == "keychain")
}
func keyringTimeoutHint(goos string) string {
switch goos {
case "darwin":
case goosDarwin:
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":
case goosLinux:
return "D-Bus SecretService may be unresponsive"
default:
return "keyring backend may be unresponsive"
@ -237,11 +241,12 @@ func openKeyring() (keyring.Keyring, error) {
// is unresponsive (e.g., gnome-keyring installed but not running).
// Use a timeout as a safety net.
if shouldUseKeyringTimeout(runtime.GOOS, backendInfo, dbusAddr) {
ring, err := openKeyringWithTimeout(cfg, keyringOpenTimeout)
if err != nil {
return nil, err
timeoutRing, timeoutErr := openKeyringWithTimeout(cfg, keyringOpenTimeout)
if timeoutErr != nil {
return nil, timeoutErr
}
return prepareKeyring(ring, backendInfo, wrapFileKeys), nil
return prepareKeyring(timeoutRing, backendInfo, wrapFileKeys), nil
}
ring, err := keyringOpenFunc(cfg)
@ -256,6 +261,7 @@ func prepareKeyring(ring keyring.Keyring, backendInfo KeyringBackendInfo, wrapFi
if wrapFileKeys || isFileKeyring(ring) {
ring = newFileSafeKeyring(ring)
}
if shouldUseKeyringOperationTimeout(runtime.GOOS, backendInfo) {
ring = newTimeoutKeyring(ring, keyringOpenTimeout, keyringTimeoutHint(runtime.GOOS))
}

3
internal/secrets/timeout_keyring.go
View File

@ -28,6 +28,7 @@ func withKeyringTimeout[T any](timeout time.Duration, operation string, hint str
}
ch := make(chan result, 1)
go func() {
value, err := fn()
ch <- result{value: value, err: err}
@ -64,6 +65,7 @@ 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
}
@ -71,6 +73,7 @@ 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
}

2
internal/secrets/timeout_keyring_test.go
View File

@ -55,6 +55,7 @@ func TestKeyringTimeoutHint(t *testing.T) {
func TestTimeoutKeyringTimesOutOperations(t *testing.T) {
block := make(chan struct{})
t.Cleanup(func() { close(block) })
ring := newTimeoutKeyring(&blockingKeyring{block: block}, 10*time.Millisecond, keyringTimeoutHint("darwin"))
@ -63,6 +64,7 @@ func TestTimeoutKeyringTimesOutOperations(t *testing.T) {
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)
}