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.

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

credential helpers don't work on Windows if sh is not in the path #274

Closed
mcgoo opened this issue Dec 5, 2017 · 5 comments
Closed

credential helpers don't work on Windows if sh is not in the path #274

mcgoo opened this issue Dec 5, 2017 · 5 comments

Comments

@mcgoo
Copy link

mcgoo commented Dec 5, 2017

https://github.com/alexcrichton/git2-rs/blob/047c984d3b934e536fd4cdd54a691ceb233ce92a/src/cred.rs#L293

This requiressh to be in the path. My recent git installation from git-scm.org seems to deliberately not add it. Changing the code to call the helper using cmd /c makes it work for my specific case which ends up calling git credential-store get. This discussion in cargo-test rust-lang/cargo#4609 seems to suggest that using sh to run the helper is to allow shell expansion, so not calling it might not be an option. (The git manpage does say "The resulting string is executed by the shell.")

It's not clear how to fix this. Always using cmd /c will probably break someone's path handling. Falling back from sh -c to cmd /cif sh is not available will probably make it even harder to work out what is going on if you are using a helper that contains a path and you get a failure.

I'd like to have a solution that does something automatic like bypassing sh if it is not required, but maybe the right thing to do is to just display a nicer error if a credential helper is set up but sh is not in the path?

End user failure: rust-lang/cargo#3487
cc: @ehuss

@alexcrichton
Copy link
Member

I think that credential helpers are defined by git as shell scripts, right? Is there anything this library can do if sh isn't available?

@ehuss
Copy link
Contributor

ehuss commented Dec 5, 2017

My personal preference is that git2-rs would behave the same as the standard git command. It should only try to use sh if there are shell-like characters in the credential helper (see git/run-command.c for how they detect it). I think that would avoid the need for sh in most use cases. In the rare case someone needs something more sophisticated, they can just ensure that sh is in their PATH (typically from C:\Program Files\Git\usr\bin).

I think it would also be useful if git2-rs would relay a better error message when the credential helper fails. Currently the error gets eaten (cred.rs) which makes it difficult to diagnose what went wrong.

@mcgoo
Copy link
Author

mcgoo commented Dec 6, 2017

Yeah, can't do much without sh. Would you accept a PR for any of the following?

  1. Better error message
  2. Asking git where its sh is. (It knows where its bundled sh is but I haven't found an option for printing it.)
  3. Emulate gits heuristic and don't use sh -c if the command doesn't look like it needs it

@alexcrichton
Copy link
Member

@ehuss oh nice that makes sense to me! Seems reasonable to me that we could have a pretty simplistic parser to handle that.

@mcgoo all of those sound great to me! I'd probably hold off on the asking git where sh is if we have basic emulation, but we can always add that later if need be.

@mcgoo
Copy link
Author

mcgoo commented Dec 6, 2017

Not necessarily a dealbreaker, but it turns out that git/run-command.c punts to sh -c if the command contains a space. Splitting the args will have to be handled in git2-rs.

 bash
  \_ git clone http://gitlab.example.com/example/repo.git
      \_ git-remote-http origin http://gitlab.example.com/example/repo.git
          \_ /bin/sh -c git credential-store get git credential-store get
              \_ git credential-store get
                  \_ /bin/sh /usr/lib/git-core/git-credential-store get
                      \_ sleep 60

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants