Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

lfs 2.3.x seems to prefer askpass to credential helper #2634

Closed
zvinless opened this issue Sep 30, 2017 · 19 comments
Closed

lfs 2.3.x seems to prefer askpass to credential helper #2634

zvinless opened this issue Sep 30, 2017 · 19 comments

Comments

@zvinless
Copy link

zvinless commented Sep 30, 2017 •
edited

When pushing/pulling lfs files with both a credential helper (osxkeychain) and a GIT_ASKPASS environment variable set, it seems like lfs tries to use the askpass program. From what I understand this is incongruent with git (though I am honestly not very familiar with how git deals with credentials).

Steps:

  1. Have valid credentials in macOS keychain such that you can clone your own GitHub repo
  2. Execute git config --global credential.helper osxkeychain to set global credential helper
  3. Clone one of your GitHub repos with lfs files
  4. Set GIT_ASKPASS environment variable in ~/.bash_profile to an empty shell script so that it will fail
  5. Commit an lfs tracked file to repo
  6. Try to execute git push
  7. Observe something along the lines of: Git credentials for [repo].git not found: fork/exec [path/to/askpass/program]: exec format error (indicating that the askpass program is getting used; don't worry that it fails)
  8. Remove GIT_ASKPASS variable from ~/.bash_profile and observe that you can now push the commit
  9. Redo these steps with a plain GitHub repo (no lfs) and observe everything works fine the first time

I first noticed this because Visual Studio Code's git integration sets the GIT_ASKPASS environment variable and provides its own askpass program, which started getting used instead of my keychain credentials exclusively since updating to git lfs 2.3.0.

env:

macOS 10.13
git-lfs/2.3.1 (GitHub; darwin amd64; go 1.9)
git version 2.14.2

Endpoint=https://github.com/zvinless/large-files.git/info/lfs (auth=basic)
LocalWorkingDir=/Users/zvinless/Desktop/large-files
LocalGitDir=/Users/zvinless/Desktop/large-files/.git
LocalGitStorageDir=/Users/zvinless/Desktop/large-files/.git
LocalMediaDir=/Users/zvinless/Desktop/large-files/.git/lfs/objects
LocalReferenceDir=
TempDir=/Users/zvinless/Desktop/large-files/.git/lfs/tmp
ConcurrentTransfers=3
TusTransfers=false
BasicTransfersOnly=false
SkipDownloadErrors=false
FetchRecentAlways=false
FetchRecentRefsDays=7
FetchRecentCommitsDays=0
FetchRecentRefsIncludeRemotes=true
PruneOffsetDays=3
PruneVerifyRemoteAlways=false
PruneRemoteName=origin
LfsStorageDir=/Users/zvinless/Desktop/large-files/.git/lfs
AccessDownload=basic
AccessUpload=basic
DownloadTransfers=basic
UploadTransfers=basic
GIT_ASKPASS=/Users/zvinless/Desktop/askpass.sh
git config filter.lfs.process = "git-lfs filter-process"
git config filter.lfs.smudge = "git-lfs smudge -- %f"
git config filter.lfs.clean = "git-lfs clean -- %f"
@technoweenie
Copy link
Contributor

It was implemented this way deliberately due to a different interpretation of the docs

git-lfs/lfsapi/creds.go

Lines 44 to 54 in 28925b7

var hs []CredentialHelper
if len(ccfg.AskPass) > 0 {
hs = append(hs, &AskPassCredentialHelper{
Program: ccfg.AskPass,
})
}
var h CredentialHelper
h = &commandCredentialHelper{
SkipPrompt: ccfg.SkipPrompt,
}

So how about this:

  • If credential.helper is configured, use git credential only
  • Otherwise, check askpass first, before using git credential. Even without a configured helper, git credential can prompt for your login. LFS will cache the results for the current LFS command.

This was referenced Oct 2, 2017
Open
Closed
Merged
@technoweenie
Copy link
Contributor

Here's a fix: #2637

And here are some builds, based on this commit, if you want to try it out:

git-lfs-darwin-amd64-2.3.1.tar.gz
git-lfs-linux-amd64-2.3.1.tar.gz
git-lfs-windows-amd64-2.3.1.zip

(let me know if you want a build for another platform, or instructions on building it yourself)

@dakotahawkins
Copy link
Contributor

@technoweenie I took a look at what git does:

credential_fill() will first go through all the helpers until it either finds a username/password or a helper tells it to quit (not sure when one would do this, it seems up to whoever wrote the helper). If a helper tells it to quit, it will die.

If none of the helpers worked, it will call gredential_getpass() which will call credential_ask_one() which will call git_prompt() which will try askpass.

So the behavior should be:

  1. Use credential helpers in the order they're defined until one succeeds or tells you to quit.
  2. If none of the credential helpers worked or told you to quit, get the askpass program and try to run that.

@zvinless
Copy link
Author

zvinless commented Oct 2, 2017

@technoweenie thanks! I can test it out tonight. @dakotahawkins's explanation lines up with what I was seeing while testing. It'd be nice if it were that explicit in the docs 😄.

@technoweenie
Copy link
Contributor

@dakotahawkins Thanks for finding the source. Is that the source for git credential?

@dakotahawkins
Copy link
Contributor

@technoweenie I don't know how much of git credential it is exactly, but those must be the functions that do its work, I think I just started by searching the whole repo for ASKPASS and there wasn't anything besides that stuff and tests.

@zvinless
Copy link
Author

zvinless commented Oct 3, 2017

@technoweenie the Mac build works great for me. Defining the GIT_ASKPASS environment variable doesn't affect anything if my keychain credentials are valid.

@technoweenie
Copy link
Contributor

Looks like the actual git-credential command is here. It seems strange that LFS has to reimplement askpass support, when it seems to be handled here already.

At any rate, seems like 2.3.2 is a go 👍

@mrbobbybobberson
Copy link

Nice! I'm happy for this change. We use SourceTree which always sets git_askpass, and were getting hammered by AskPass prompts even though we prefer using git credential helper. I'm looking forward to trying this out!

@wouterh-dev
Copy link

@technoweenie With git-lfs 2.3.4 this appears to be still be a problem. My credential helper is configured in /etc/gitconfig like this:

[credential "https://bitbucket.org/"]
        helper = /opt/tools/git-credential-helper

And SSH_ASKPASS is set to /usr/libexec/openssh/gnome-ssh-askpass, which keeps getting invoked, instead of the credential helper which git itself uses just fine. And versions of git-lfs prior to 2.3.1 also worked fine.

@technoweenie
Copy link
Contributor

I can't replicate it:

$ rm -rf .git/lfs/objects && SSH_ASKPASS="echo fail" GIT_TRACE=1 git lfs fetch --all
09:02:12.441099 git.c:560               trace: exec: 'git-lfs' 'fetch' '--all'
09:02:12.441671 run-command.c:626       trace: run_command: 'git-lfs' 'fetch' '--all'
09:02:12.450307 trace git-lfs: run_command: 'git' version
09:02:12.462428 trace git-lfs: run_command: 'git' config -l
Scanning for all objects ever referenced...
09:02:12.468463 trace git-lfs: run_command: git rev-list --objects --all --
09:02:12.474807 trace git-lfs: run_command: git cat-file --batch
✔ 8 objects found
Fetching objects...
09:02:12.519268 trace git-lfs: run_command: 'git' -c filter.lfs.smudge= -c filter.lfs.clean= -c filter.lfs.process= -c filter.lfs.required=false rev-parse HEAD --symbolic-full-name HEAD
09:02:12.525192 trace git-lfs: run_command: 'git' config branch.foo.remote
09:02:12.537911 trace git-lfs: tq: running as batched queue, batch size of 100
09:02:12.537958 trace git-lfs: fetch bin/again.bin [9252a75c942da16f7b52cab752797dea4fca18474db9d7eff102842a459b25b3]
09:02:12.538072 trace git-lfs: tq: sending batch of size 8
09:02:12.538368 trace git-lfs: api: batch 8 files
09:02:12.578619 trace git-lfs: HTTP: POST https://github.com/github/git-lfs-test.git/info/lfs/objects/batch
09:02:12.879606 trace git-lfs: HTTP: 401
09:02:12.879808 trace git-lfs: HTTP: {"message":"Requires authentication","documentation_url":"https://github.com/contact"}
09:02:12.879949 trace git-lfs: setting repository access to basic
09:02:12.879972 trace git-lfs: run_command: 'git' config --replace-all lfs.https://github.com/github/git-lfs-test.git/info/lfs.access basic
09:02:12.894348 trace git-lfs: api: http response indicates "basic" authentication. Resubmitting...
09:02:12.894427 trace git-lfs: creds: git credential fill ("https", "github.com", "github/git-lfs-test")
09:02:12.934722 trace git-lfs: Filled credentials for https://github.com/github/git-lfs-test
09:02:12.934805 trace git-lfs: HTTP: POST https://github.com/github/git-lfs-test.git/info/lfs/objects/batch
09:02:13.050648 trace git-lfs: HTTP: 200

If I disable credential.helper, it fails as expected:

$ SSH_ASKPASS="echo fail" GIT_TRACE=1 git -c credential.helper= lfs fetch --all
10:40:09.571423 git.c:560               trace: exec: 'git-lfs' 'fetch' '--all'
10:40:09.571940 run-command.c:626       trace: run_command: 'git-lfs' 'fetch' '--all'
10:40:09.581376 trace git-lfs: run_command: 'git' version
10:40:09.593710 trace git-lfs: run_command: 'git' config -l
Scanning for all objects ever referenced...
10:40:09.600089 trace git-lfs: run_command: git rev-list --objects --all --
10:40:09.605327 trace git-lfs: run_command: git cat-file --batch
✔ 8 objects found
Fetching objects...
10:40:09.613721 trace git-lfs: run_command: 'git' -c filter.lfs.smudge= -c filter.lfs.clean= -c filter.lfs.process= -c filter.lfs.required=false rev-parse HEAD --symbolic-full-name HEAD
10:40:09.619701 trace git-lfs: run_command: 'git' config branch.foo.remote
10:40:09.631703 trace git-lfs: tq: running as batched queue, batch size of 100
10:40:09.631728 trace git-lfs: fetch bin/again.bin [9252a75c942da16f7b52cab752797dea4fca18474db9d7eff102842a459b25b3]
10:40:09.631739 trace git-lfs: fetch bin/b.bin [0263829989b6fd954f72baaf2fc64bc2e2f01d692d4de72986ea808f6e99813f]
10:40:09.631773 trace git-lfs: fetch bin/hi.bin [98ea6e4f216f2fb4b69fff9b3a44842c38686ca685f3f55dc48c5d3fb1107be4]
10:40:09.631825 trace git-lfs: fetch gif/atom-undo.gif [b9f86fab477109565871ced361ba69f2425a91fbe6057fa7a9629a8d536d7c71]
10:40:09.631860 trace git-lfs: fetch gif/droidtocat.gif [d1c8fab51418ef587fcf5dd8e73c60b9700704e2f8f5292ea12ec27c285b23a3]
10:40:09.631871 trace git-lfs: fetch jpg/kabuki-tattoo-left.JPG [e1732d149d262963996ca77a47a5d3c51c85c6e97187afad370a8bef2828b6b7]
10:40:09.631880 trace git-lfs: fetch png/render.png [55d51edb307ed7bf4a1bbca3867d132a07f76828d57f37e0f31f712c02ceafe0]
10:40:09.631887 trace git-lfs: fetch render.png [124733b36d098353ad16bb11646643709e87c4746e0d97913a9826829e0477a3]
10:40:09.631959 trace git-lfs: tq: sending batch of size 8
10:40:09.632236 trace git-lfs: api: batch 8 files
10:40:09.632367 trace git-lfs: creds: filling with GIT_ASKPASS: echo fail Username for "https://github.com/github/git-lfs-test"
10:40:09.632388 trace git-lfs: api error: Git credentials for https://github.com/github/git-lfs-test not found:
exec: "echo fail": executable file not found in $PATH
Git LFS: (0 of 8 files) 0 B / 879.12 KB
batch response: Git credentials for https://github.com/github/git-lfs-test not found:
exec: "echo fail": executable file not found in $PATH
error: failed to fetch some objects from 'https://github.com/github/git-lfs-test.git/info/lfs'

This is also confirmed in these integration tests.

@wouterh-dev
Copy link

wouterh-dev commented Oct 24, 2017 via email

@technoweenie
Copy link
Contributor

Doh, good call. Opened an issue here: #2686

@technoweenie
Copy link
Contributor

technoweenie commented Oct 27, 2017 •
edited

Hey all, I made some more fixes to the credential helper in #2695, and uploaded some builds for windows, linux, and mac. I'd appreciate if anyone could try it to confirm the fixes.

  1. The askpass values are ignored if credential.helper or credential.<url>.helper are set.
  2. If all credential helper values are empty and the askpass script is attempted, but returns an error, only a warning is issued. LFS should try the credential helper anyway, which will just prompt you at the command line.

EDIT: clarified second point to make more sense.

@wouterh-dev
Copy link

@technoweenie Tested & confirmed that it resolves the issue I've reported

@wouterh-dev wouterh-dev mentioned this issue Oct 27, 2017
Merged
6 tasks
@zvinless
Copy link
Author

The mac version from #2695 works as expected for me (didn't try anything new, but no regressions!). I'm a little confused about the 2nd part, though; what exactly is the order that things happen? My understanding is it'll go (simplified):

  1. credential helper
  2. askpass
  3. basic command line prompt

Disabling my credential helper did result in a prompt, but I didn't see a warning?

@technoweenie
Copy link
Contributor

@zvinless Well, it is confusing because I left out some key words :)

The symptom you described in #2634 is that users had an invalid askpass set, causing LFS operations to fail. We determined that your interpretation of the docs was correct, and that LFS was using the wrong order. Now, I think the correct order is:

  1. askpass (if credential.*.helper is undefined and one of the askpass config values are present)
  2. git credential (uses a helper if configured, or prompts on the command line)

#2695 doesn't change the order or anything, but it does recover gracefully from an askpass failure. If someone misconfigures askpass, but still doesn't have a credential helper defined, LFS will prompt you at most once per LFS command.

@zvinless
Copy link
Author

Ok, gotcha. Looks good then! 👍

@mrbobbybobberson
Copy link

@technoweenie, yep, the Windows version works for my scenario, too. Even though SourceTree sets the git_askpass environment variable before launching git, git-lfs now gives preference to our credential helper settings, so we no longer get hammered with the SourceTree askpass prompt. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@technoweenie @dakotahawkins @wouterh-dev @mrbobbybobberson @zvinless and others