fix(keychain): unlock the item on relock, not just the collection

store/keychain/internal/go-keychain/secretservice/secretservice.go

@@ -110,7 +110,15 @@ func NewService(ctx context.Context) (*SecretService, error) {
 	}
 	signalCh := make(chan *dbus.Signal, 16)
 	conn.Signal(signalCh)
-	_ = conn.AddMatchSignal(dbus.WithMatchOption("org.freedesktop.Secret.Prompt", "Completed"))
+	// Without this subscription the bus never delivers Prompt.Completed and
+	// PromptAndWait times out on every real (non-null) prompt.
+	if err := conn.AddMatchSignal(
+		dbus.WithMatchInterface("org.freedesktop.Secret.Prompt"),
+		dbus.WithMatchMember("Completed"),
+	); err != nil {
+		_ = conn.Close()
+		return nil, fmt.Errorf("failed to subscribe to prompt completion signals: %w", err)
+	}
 	return &SecretService{conn: conn, signalCh: signalCh, sessionOpenTimeout: DefaultSessionOpenTimeout}, nil
 }
 

store/keychain/keychain_linux.go

@@ -277,8 +277,9 @@ const (
 // synchronisation, so tests that swap it must not run in parallel.
 var sleepFn = time.Sleep
 
-// withRelockRetry runs a collection operation, retrying it with exponential
-// backoff when the secret service rejects it because the collection is locked.
+// withRelockRetry runs a collection or item operation, retrying it with
+// exponential backoff when the secret service rejects it because the
+// collection is locked.
 //
 // The store dials a fresh D-Bus connection for every operation and closes it on
 // return. gnome-keyring scopes an unlock to the session that performed it, so
@@ -291,20 +292,26 @@ var sleepFn = time.Sleep
 // between the check and the call, so we react to the authoritative signal — the
 // operation's own locked error — by unlocking again and retrying.
 //
+// itemPaths are unlocked alongside collectionPath. The spec allows an item to
+// be locked independently of its collection (e.g. KeePassXC's per-item access
+// confirmation), so operations that target a known item must pass its path;
+// omit it only when none exists yet (CreateItem).
+//
 // In the common case this is the passwordless auto-unlock path (e.g. the
 // PAM-unlocked login keyring), where Unlock returns the null prompt and asks
 // the user for nothing. withRelockRetry cannot itself prove the keyring is
 // passwordless, so on a password-protected keyring a retry could surface an
 // authentication prompt; the bounded retry count and backoff keep that to a
 // handful of spaced-out prompts at worst, and a dismissed prompt makes Unlock
 // return an error that aborts the loop immediately rather than re-prompting.
-func withRelockRetry(service secretService, collectionPath dbus.ObjectPath, op func() error) error {
+func withRelockRetry(service secretService, collectionPath dbus.ObjectPath, op func() error, itemPaths ...dbus.ObjectPath) error {
 	err := op()
 	delay := relockRetryBaseDelay
+	unlockPaths := append([]dbus.ObjectPath{collectionPath}, itemPaths...)
 	for attempt := 0; attempt < maxRelockRetries && isLockedDBusError(err); attempt++ {
 		sleepFn(delay)
 		delay = min(delay*2, relockRetryMaxDelay)
-		if unlockErr := service.Unlock([]dbus.ObjectPath{collectionPath}); unlockErr != nil {
+		if unlockErr := service.Unlock(unlockPaths); unlockErr != nil {
 			// Surface why the retry stopped while preserving errors.Is on the
 			// underlying Unlock error (e.g. a dismissed prompt). The original
 			// locked error is intentionally dropped: the failed unlock is the
@@ -368,7 +375,7 @@ func (k *keychainStore[T]) Delete(ctx context.Context, id store.ID) error {
 
 	return withRelockRetry(service, objectPath, func() error {
 		return service.DeleteItem(items[0])
-	})
+	}, items[0])
 }
 
 func (k *keychainStore[T]) Get(ctx context.Context, id store.ID) (store.Secret, error) {
@@ -426,7 +433,7 @@ func (k *keychainStore[T]) Get(ctx context.Context, id store.ID) (store.Secret,
 		var getErr error
 		value, getErr = service.GetSecret(items[0], *session)
 		return getErr
-	})
+	}, items[0])
 	if err != nil {
 		return nil, err
 	}
@@ -594,7 +601,7 @@ func (k *keychainStore[T]) Save(ctx context.Context, id store.ID, secret store.S
 	primary := items[0]
 	if err := withRelockRetry(service, objectPath, func() error {
 		return service.SetItemSecret(primary, sessSecret)
-	}); err != nil {
+	}, primary); err != nil {
 		return err
 	}
 	_ = service.SetItemAttributes(primary, attributes)
@@ -605,7 +612,7 @@ func (k *keychainStore[T]) Save(ctx context.Context, id store.ID, secret store.S
 		// exists to drain (see withRelockRetry and issue #446).
 		_ = withRelockRetry(service, objectPath, func() error {
 			return service.DeleteItem(dup)
-		})
+		}, dup)
 	}
 
 	return nil
@@ -631,7 +638,7 @@ func (k *keychainStore[T]) loadSecret(
 		var getErr error
 		value, getErr = svc.GetSecret(itemPath, *session)
 		return getErr
-	})
+	}, itemPath)
 	if err != nil {
 		return nil, err
 	}

store/keychain/keychain_linux_test.go

@@ -74,6 +74,8 @@ type fakeService struct {
 	unlockCalls          int
 	unlockErr            error
 
+	lastUnlockPaths []dbus.ObjectPath
+
 	// availableErr, when set, is returned by Available so a test can drive the
 	// eager-probe failure paths in New. The zero value (nil) reports the backend
 	// as available, so every existing test that constructs a store via
@@ -110,8 +112,9 @@ func (f *fakeService) OpenSession(kc.AuthenticationMode) (*kc.Session, error) {
 	return &kc.Session{Mode: kc.AuthenticationInsecurePlain}, nil
 }
 func (f *fakeService) CloseSession(*kc.Session) {}
-func (f *fakeService) Unlock([]dbus.ObjectPath) error {
+func (f *fakeService) Unlock(items []dbus.ObjectPath) error {
 	f.unlockCalls++
+	f.lastUnlockPaths = items
 	return f.unlockErr
 }
 
@@ -333,6 +336,8 @@ func TestKeychainSaveRetriesWhenSetSecretRelocks(t *testing.T) {
 	assert.Equal(t, []dbus.ObjectPath{"/item/a"}, fake.setSecretItems,
 		"the secret must be written in place once the relock clears")
 	assert.Equal(t, 2, fake.unlockCalls, "exactly one Unlock per relock retry")
+	assert.Contains(t, fake.lastUnlockPaths, dbus.ObjectPath("/item/a"),
+		"the retry must unlock the item itself, not only its collection")
 }
 
 // TestKeychainSaveCollapseRetriesWhenDeleteRelocks is the unit-level counterpart
@@ -354,6 +359,8 @@ func TestKeychainSaveCollapseRetriesWhenDeleteRelocks(t *testing.T) {
 		"the duplicate must be collapsed once the relock clears")
 	assert.Equal(t, 3, fake.deleteCalls, "two locked failures then one success")
 	assert.Equal(t, 2, fake.unlockCalls, "exactly one Unlock per relock retry")
+	assert.Contains(t, fake.lastUnlockPaths, dbus.ObjectPath("/item/b"),
+		"the retry must unlock the duplicate item itself, not only its collection")
 }
 
 // TestKeychainSaveStopsRetryingAfterMaxRelocks asserts the retry is bounded: a
@@ -389,6 +396,8 @@ func TestKeychainGetRetriesWhenCollectionRelocks(t *testing.T) {
 
 	assert.Equal(t, 3, fake.getSecretCalls, "two locked failures then one success")
 	assert.Equal(t, 2, fake.unlockCalls, "exactly one Unlock per relock retry")
+	assert.Contains(t, fake.lastUnlockPaths, dbus.ObjectPath("/org/freedesktop/secrets/collection/login/1"),
+		"the retry must unlock the item itself, not only its collection")
 }
 
 // TestKeychainFilterRetriesWhenCollectionRelocks covers the read path reached