fix: close the add-then-select visibility race in Keychain/libsecret backends - #30
Merged
Merged
Conversation
…ret backends AddCredentialAsync returned as soon as the OS store accepted the item, but a just-added item isn't always immediately visible to a follow-up query on the macOS Keychain (and, less often, the Linux Secret Service) under concurrent access. A caller doing add-then-select/delete — which AddCredentialCommand does in production when it offers to activate the new credential — could therefore race the item's own appearance and have the follow-up lookup miss it, returning false. Fix it at the source: both backends now confirm the item is queryable (a bounded ~500ms poll) before AddCredentialAsync returns, so any subsequent operation sees it. Best-effort — returns after the bounded wait regardless, leaving genuine failures to the caller's own lookup. This also removes the root cause behind the test-side retry hardening added earlier (kept as defence-in-depth). Experimental backends only; the file backend has no such window. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
AddCredentialAsyncreturned as soon as the OS store accepted the item, but a just-added item isn't always immediately visible to a follow-up query on the macOS Keychain (SecItemAdd→SecItemCopyMatchinglag), and occasionally the Linux Secret Service, under concurrent access. So a caller doing add-then-select/delete — whichAddCredentialCommanddoes in production when it offers to activate the new credential — could race the item's own appearance and have the follow-up lookup miss it, returningfalse.This is the same race we'd been hardening test-side (delete/export/select assertions); it's a genuine latent bug in the experimental backends, not just a test artifact.
The fix
Both backends now confirm the item is queryable before
AddCredentialAsyncreturns (a bounded ~500ms poll of the by-account lookup). Best-effort: it returns after the bounded wait regardless, leaving any genuine failure to the caller's own lookup. The file backend has no such window and is untouched.Notes
Verified: clean build (flag on, both TFMs); 82/82 libsecret + file tests pass locally (libsecret exercises the new confirm-visible path for real).
🤖 Generated with Claude Code