Only erase credentials that match the username/password if specified #25
Conversation
This file contains 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
Only erase stored credentials when the user name and password values match those passed on standard input, if any. When no user name and/or password are given on standard input we skip checking the stored value and just erase the credential. This is an important but possibly subtle behaviour to help prevent erroneous erasure of credentials. Calls to `git-credential fill` should immediately be followed by `git-credential approve` or `reject`. If there are concurrent processes that have called `fill` but not yet `approve` or `reject`, there's a gap when another (faster) process could have completed the `fill+approve` combination and have stored a different credential value than what the first process got from `fill`. If the first process fails and calls `reject` the valid credential as `approve`-ed by the second process would be deleted. This change helps prevent such a scenario (although there is still a small gap inside the GCM erase command itself between reading the OS's credential store and deleting from it - we could look at some system-wide lock around the credential store if this is deemed a problem later).
jrbriggs
reviewed
Feb 17, 2019
jrbriggs
approved these changes
Feb 18, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Only erase stored credentials when the user name and password values match those passed on standard input, if any. When no user name and/or password are given on standard input we skip checking the stored value and just erase the credential.
This is an important but possibly subtle behaviour to help prevent erroneous erasure of credentials.
Calls to
git-credential fillshould immediately be followed bygit-credential approveorreject. If there are concurrent processes that have calledfillbut not yetapproveorreject, there's a gap when another (faster) process could have completed thefill+approvecombination and have stored a different credential value than what the first process got fromfill.If the first process fails and calls
rejectthe valid credential asapprove-ed by the second process would be deleted. This change helps prevent such a scenario (although there is still a small gap inside the GCM erase command itself between reading the OS's credential store and deleting from it - we could look at some system-wide lock around the credential store if this is deemed a problem later).Fixes #24