Merged
Conversation
…et in the git config Provide the URL path to 'git credential fill' for credential helpers that need the path to return the right credentials Use the objects API URL for credentials so there is a stable path passed to the credential helper for caching
…t users don't have to cache the same credentials twice
chrisd8088
added a commit
to manturovDan/git-lfs
that referenced
this pull request
Mar 5, 2026
In subsequent commits in this PR we intend to add new tests to validate the changes we made to the Git LFS client in prior commits of this PR. We plan to add these tests to both the "t/t-askpass.sh" and the "t/t-credentials.sh" scripts in our shell test suite. Before we add our new tests, we first update the existing "credentials with useHttpPath, with wrong password" test in the "t/t-credentials.sh" script, and add a similar test to the "t/t-askpass.sh" script, which at present does not contain a test with an invalid set of credentials. We will then be able to use these tests as templates for the additional tests we expect to add in future commits. First, we revise the "credentials with useHttpPath, with wrong password" test to correct a check which at present will always trivially succeed. After the test runs the "git push" command, it attempts to confirm that the command failed to push a Git LFS object by checking that the command did not output a final progress message stating that the object was uploaded. To verify the lack of this message, the test uses the -c option of the grep(1) command and then checks that the returned value is zero. When the original version of this test was first introduced in commit 69aee53 of PR git-lfs#561, the "grep" command searched for the pattern "(1 of 1 files)" in the output of the "git push" command, which was a valid check given the format of the Git LFS client's progress messages at the time. We then revised the format of the client's progress messages in commit db4e3b2 of PR git-lfs#2811, and updated many of the "grep" commands in our test suite to reflect the new format at the same time. Most of these alterations were made correctly, but in the "credentials with useHttpPath, with wrong password" test the "grep" command's pattern was changed to "Uploading LFS objects: 100% (1/1), 0 B". Even if the "git push" command in the test were to unexpectedly succeed, the trailing "0 B" portion of this pattern would still not match any lines in the command's output, since the size of the test Git LFS object to be pushed is actually one byte. Thus this check will always pass, regardless of the success or failure of the "git push" command. To address this problem we simply remove the trailing portion of the pattern, as we only need to confirm that the "git push" command's output does not match the pattern "100% (1/1)". We then add another, final check to the test which uses a different technique to verify that the "git push" command did not upload a Git LFS object. Since we have already computed the OID of the object, we can call our refute_server_object() test assertion function to confirm that the object does not exist on the Git LFS remote. This is a more direct and less fragile method of checking the behaviour of the "git push" command than parsing its output messages, and should help avoid any future regressions of the type we have fixed in this commit. As well as making these changes to the "credentials with useHttpPath, with wrong password" test, we also add several new checks which use "grep" commands to ascertain that HTTP authorization errors occurred during the "git push" command. Specifically, we check that the client output two error messages, one after its request to the Git LFS lock verification API endpoint received a 403 status code response, and another after the client's request to the Git LFS Batch API endpoint received a 403 status code response. While adding these extra checks we also adjust the whitespace in the test, and we drop an "echo" statement whose value is minimal, even for debugging purposes. Finally, we replicate portions of the design of this test to create a new "askpass: push with core.askPass and wrong password" test in our "t/t-askpass.sh" script. This test establishes the same conditions as those from the existing "askpass: push with core.askPass" test, except that we set the LFS_ASKPASS_PASSWORD environment variable to contain an invalid password. As in our revised "credentials with useHttpPath, with wrong password" test, our new "askpass: push with core.askPass and wrong password" test then verifies the "git push" command made with the invalid credentials did not upload a Git LFS object, and that two 403 status code responses were received, one from the lock verification API endpoint and one from the Batch API endpoint.
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
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.
Trying to spike out some tests for the git credential behavior. This is so we can find out if we break the credential flow, and test changes to it like #554.