From 8f21b3444cdf3cbe785ec1669ccc01e5881c5676 Mon Sep 17 00:00:00 2001 From: Alano Terblanche <18033717+Benehiko@users.noreply.github.com> Date: Mon, 3 Aug 2026 14:02:50 +0200 Subject: [PATCH] fix(keychain)!: return empty map from Filter on Linux when nothing matches The Linux Secret Service backend's Filter treated zero matches as store.ErrCredentialNotFound, both when the collection search returned no items and when items existed but none survived pattern matching. The macOS and Windows backends return an empty map with a nil error in the same situation, and 4a163cd already aligned GetAllMetadata the same way. Drop both guards and document the contract on store.Store.Filter: a pattern matching nothing is a valid empty result, not a miss. ErrCredentialNotFound stays reserved for single-item lookups. BREAKING CHANGE: Filter on Linux no longer returns store.ErrCredentialNotFound when nothing matches; callers checking for that error on empty filter results must handle an empty map instead. Add fake-driven regression tests for the empty-collection and no-pattern-match cases. Co-Authored-By: Claude Fable 5 --- store/keychain/keychain_linux.go | 8 -------- store/keychain/keychain_linux_test.go | 26 ++++++++++++++++++++++++++ store/store.go | 3 +++ 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/store/keychain/keychain_linux.go b/store/keychain/keychain_linux.go index 7be4241d..161968c9 100644 --- a/store/keychain/keychain_linux.go +++ b/store/keychain/keychain_linux.go @@ -689,10 +689,6 @@ func (k *keychainStore[T]) Filter(ctx context.Context, pattern store.Pattern) (m return nil, fmt.Errorf("failed to search collection: %w", err) } - if len(itemPaths) == 0 { - return nil, store.ErrCredentialNotFound - } - credentials := make(map[store.ID]store.Secret) for _, itemPath := range itemPaths { attributes, err := service.GetAttributes(itemPath) @@ -728,9 +724,5 @@ func (k *keychainStore[T]) Filter(ctx context.Context, pattern store.Pattern) (m credentials[secretID] = secret } - if len(credentials) == 0 { - return nil, store.ErrCredentialNotFound - } - return credentials, nil } diff --git a/store/keychain/keychain_linux_test.go b/store/keychain/keychain_linux_test.go index f42fa633..6cc8c589 100644 --- a/store/keychain/keychain_linux_test.go +++ b/store/keychain/keychain_linux_test.go @@ -415,6 +415,32 @@ func TestKeychainFilterRetriesWhenCollectionRelocks(t *testing.T) { assert.Equal(t, 2, fake.unlockCalls, "exactly one Unlock per relock retry") } +func TestKeychainFilterEmpty(t *testing.T) { + fake := &fakeService{} // no items -> empty search + withFakeService(t, fake) + + ks := setupKeychain(t, nil) + creds, err := ks.Filter(t.Context(), store.MustParsePattern("**")) + require.NoError(t, err) + assert.NotNil(t, creds) + assert.Empty(t, creds) +} + +func TestKeychainFilterNoMatch(t *testing.T) { + fake := &fakeService{ + items: []dbus.ObjectPath{"/org/freedesktop/secrets/collection/login/1"}, + attributes: kc.Attributes{"id": "com.test.test/test/bob"}, + } + withFakeService(t, fake) + + ks := setupKeychain(t, nil) + creds, err := ks.Filter(t.Context(), store.MustParsePattern("com.test.test/test/alice")) + require.NoError(t, err) + assert.NotNil(t, creds) + assert.Empty(t, creds) + assert.Equal(t, 0, fake.getSecretCalls, "non-matching item must not be loaded") +} + // The real-keychain dedup tests use their own service group/name so their items // are namespace-isolated from TestKeychain (which shares com.test.test/test). // GetAllMetadata/Filter search by {service:group, service:name}, so a leaked diff --git a/store/store.go b/store/store.go index 28c4b640..211bf505 100644 --- a/store/store.go +++ b/store/store.go @@ -93,6 +93,9 @@ type Store interface { Upsert(ctx context.Context, id ID, secret Secret) error // Filter returns a map of secrets based on a [Pattern]. // + // A pattern matching nothing is a valid empty result: implementations + // return an empty (non-nil) map with a nil error, not ErrCredentialNotFound. + // // Secrets returned will have both [Secret.SetMetadata] and [Secret.Unmarshal] // called; in that order. Any error produced by any of them would result in // an early return with a nil secrets map.