Re: Apparent bug in credential tool running...

[Jeff King <peff@peff.net>]

On Wed, Mar 28, 2018 at 06:26:08PM +0000, Erik Brady -X (brady - ROBERT HALF INTERNATIONAL INC at Cisco) wrote:

> The location of the problem is in credential.c, run_credential_helper()... this code:
> 
>    ...
>     fp = xfdopen(helper.in, "w");
>     credential_write(c, fp);
>     fclose(fp);
>    ..
> 
> Which I think needs to become something like this:
> 
>     fp = xfdopen(helper.in, "w");
>     sigchain_push(SIGPIPE, SIG_IGN);
>     credential_write(c, fp);
>     fclose(fp);
>     sigchain_pop(SIGPIPE);
> 
> The basics are that we wrote a credential helper in Go and, for the
> store action, it simply exits 0.  It is fast.  This is similar to the
> example here:

Yeah, that makes sense. Generally a pipe buffer would be plenty to hold
a credential, but we're racing against whether the other process exits
before we even write anything, so it's bound to fail eventually in a
racy way.

I don't think we've ever made a promise[1] about whether credential
helpers have to read their input, but it makes sense to me for Git to be
friendly and handle this case. We've done similar things for hooks.

Curiously, I have a very similar helper myself, which I did as an inline
shell snippet in my ~/.gitconfig:

  [credential "https://github.com"]
  username = peff
  helper = "!f() { test $1 = get && echo password=`pass peff/github/oauth`; }; f"

I guess I've never lost the race because of the sheer number of
sub-processes that get spawned (shell to "pass" which is itself a shell
script, which spawns gpg -- yikes!).

Do you want to send your change as a patch? There's some guidance in
Documentation/SubmittingPatches.

-Peff

[1] I know you pulled a similar example from the Pro Git book content,
    which we mirror on git-scm.com.  The quality there is usually quite
    good, but I don't consider it as authoritative as the manpages. :)