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

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 29, 2018, 19:06 UTC
Message-ID: <xmqqk1n486o1.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180929081705.GI2174@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 9 quoted lines

> 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).

After reading that thread again, my version of summary is that

 - storing the credential obtained from a helper back to the same
   helper may be pointless at best and may incur end-user
   interaction (e.g. asking for symmetric encryption key before the
   data hits the disk), but it can be used as a "we used what you
   gave us successfully" ping-back signal.  We may not have designed
   approve() to do "store" back to the same helper by default and
   instead to do so only when asked by the helper, if we knew
   better.  But an unconditional change of behaviour will break
   existing users and helpers.

 - storing the credential obtained from a helper to a different
   helper may have security implications, and we may not have
   designed approve() to do "store" by default to all helpers if we
   knew better.  But an unconditional change of behaviour will break
   existing users and helpers.

Assuming that the above summary is accurate, I think the former is easier to solve. In the ideal end game state, we would also have "accepted" in the protocol and call the helper that gave us the accepted credential material with an earlier "get" (if the helper is updated to understand "accepted"). The "accepted" may not even need to receive the credential material as the payload.

The latter is trickier, as there are design considerations.

 - We may want to allow the helper that gives the credential back
   from the outside world upon "get" request to say "you can 'store'
   this to other helpers of this kind but not of that kind".  If we
   want to do so, we'd need to extend what is returned from the
   helper upon "get" (e.g. "get2" will give more than what "get"
   does back).

 - We may want to allow the helper that receives the credential
   material from others to behave differently depending on where it
   came from, and what other helpers done to it (e.g. even a new
   credential the end user just typed may not want to go to all
   helpers; an on-disk helper may feel "I'd take it if the only
   other helpers that responded to 'store' are the transient
   'in-core' kind, but otherwise I'd ignore").  I am not offhand
   sure what kind of flexibility and protocol extension is
   necessary.

 - We may want to be able to override the preference the helper
   makes in the above two.  The user may want to say "Only use this
   on-disk helper as a read-only source and never call 'store' on it
   to add new entries (or update an existing one)."