Do not erase an approved credential on a single blob-download rejection - #2087
Closed
tyrielv wants to merge 1 commit into
Closed
Do not erase an approved credential on a single blob-download rejection#2087tyrielv wants to merge 1 commit into
tyrielv wants to merge 1 commit into
Conversation
GVFS erases the cached credential and calls git-credential reject whenever an object-download response is 401, 400, or 302. On-demand loose-blob downloads run many at a time and share one cached credential. When one download fails - even intermittently, or for a non-auth reason misclassified as auth under load - the erase removes a credential that sibling downloads are still using successfully. The erase forces a re-authentication and can start a credential-manager consent-popup storm. Gate the erase of an already-approved credential. A credential the server has accepted at least once this generation is only erased after several consecutive rejections with no intervening success. Any successful use resets the count. A credential that has never succeeded (for example a genuinely expired token at first use) is still erased on the first rejection, so normal re-authentication is not affected. Assisted-by: Claude Opus 4.8 Signed-off-by: Tyrie Vella <tyrielv@gmail.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.
Problem
GVFS erases the cached credential and issues
git credential rejectwhenever anobject-download response is 401, 400, or 302. On-demand loose-blob downloads run
many at a time and share one cached credential. When one download fails - even
intermittently, or for a non-auth reason misclassified as an auth failure under
load - the erase removes a credential that sibling downloads are still using
successfully. The erase forces a re-authentication and can start a
credential-manager consent-popup storm.
The single-rejection erase has no protection:
GitAuthentication.RejectCredentialshad only a guard against erasing a different credential than the one that
failed. It had no consecutive-failure gate, so one bad response among many good
ones erased a valid credential.
Fix
Gate the erase of an already-approved credential. A credential the server has
accepted at least once this generation is only erased after several consecutive
rejections with no intervening success. Any successful use resets the count.
A credential that has never succeeded (for example a genuinely expired token at
first use) is still erased on the first rejection, so normal re-authentication is
not affected.
The gate runs after the existing reload-from-store check, so a credential that
genuinely changed underneath us is still detected and adopted rather than
deferred.
Tests
Adds unit tests for the new behavior: an approved credential survives up to the
threshold, a success between rejections resets the count, and a never-approved
credential still erases on the first rejection. The full unit-test suite passes.