Re: [PATCH] Improvement to only call Git Credential Helper once

From: Jeff King <peff@peff.net>
Date: Sep 29, 2018, 08:17 UTC
Message-ID: <20180929081705.GI2174@sigill.intra.peff.net>
In-Reply-To: <20180928163716.29947-1-khubert@gmail.com>

On Fri, Sep 28, 2018 at 12:37:16PM -0400, Kyle Hubert wrote:

Show 11 quoted lines

> When calling the Git Credential Helper that is set in the git config,
> the get command can return a credential. Git immediately turns around
> and calls the store command, even though that credential was just
> retrieved by the Helper. This creates two side effects. First of all,
> if the Helper requires a passphrase, the user has to type it in
> twice. Secondly, if the user has a number of helpers, this retrieves
> the credential from one service and writes it to all services.
> 
> This commit introduces a new field in the credential struct that
> detects when the credential was retrieved using the Helper, and early
> exits when called to store the credential.

Wow, what's old is new again. Here's more or less the same patch from 2012:

  https://public-inbox.org/git/20120407033417.GA13914@sigill.intra.peff.net/

Unfortunately, some people seem to rely on this multi-helper behavior. I recommend reading the whole thread, as there are some interesting bits in it (that I had always meant to return to, but somehow 6 years went by).

I'm not entirely opposed to breaking the current behavior in the name of better security (namely not unexpectedly propagating credentials), but it would be nice if we provided better tools for people to let their helpers interact (like the credential-wrap thing I showed in that thread).

> ---
>  credential.c | 8 +++++++-
>  credential.h | 3 ++-
>  2 files changed, 9 insertions(+), 2 deletions(-)

I know your patch is right, because it's almost identical to mine. :) (Mine didn't use the "retrieved" flag in the middle, but just set "approved" directly).

If we do go this route, though, we might want to steal the test from that earlier round.

-Peff