Re: [PATCH] credential: add nocache option to the credentials API

[Jeff King <peff@peff.net>]

On Mon, Jul 22, 2019 at 10:30:52AM -0700, Masaya Suzuki wrote:

> > In that patch, I essentially proposed making all gathered credentials as
> > nocache. That's a more secure default (though in some cases less
> > convenient).
> >
> > It did break a case Shawn had of caching the result of another helper. I
> > showed some options there for providing a mechanism to chain helpers
> > together explicitly.
> 
> I think that it's better to make it nocache by default. Having one
> helper produce a credential and having another cache it looks storage.
> But since this is the current behavior, I'm OK with just keeping
> nocache an option. It's backward compatible.

Yeah, I think that make sense.

> > We also discussed helpers passing out an explicit ttl. That's a more
> > general case of your nocache flag (i.e., ttl=0 covers that case, but we
> > could additionally pass "ttl" to the cache helper to let it be smarter).
> 
> TTL sounds like it's a generalized version. It might be a bit awkward
> because the existing credential helpers that don't support TTL would
> anyway cache the credentials. I think in practice the password saving
> feature is mainly used by those password management software (like
> git-credential-osxkeychain), and they wouldn't support a short-lived
> credential. Just having nocache seems fine to me. As you said, if
> needed, "ttl" can be added and "nocache" can be just a shorthand of
> "ttl=0".

I was thinking that Git itself could treat "ttl=0" specially, the same
as your nocache, and avoid passing it along to any helpers during the
approve stage. That would make it exactly equivalent to your patch
(modulo the name change).

> > Here we're disallowing a "nocache" credential from being passed to _any_
> > helper, whether it's caching or not. It could be storing permanently,
> > though perhaps that's semantic nitpicking (if it's not to be cached, it
> > probably shouldn't be stored permanently either). Other helpers could in
> > theory be doing something else with the data, though in practice I doubt
> > here are any uses beyond debugging.
> 
> I cannot think of a usage either. If there's a good usage, I would
> change this, but if it's for debugging, it's better to be done with
> those debugging features (like GIT_TRACE_CURL). Note that this is
> called only when the credential is successfully used. We probably want
> to use such debugging feature for the credentials that are not
> successfully used.

Yeah, I don't think debugging is worth caring about here. As you say, we
can dump the data readily through other means. I was more wondering if
there was some legitimate use where a helper wanted to see (but not
store!) an existing credential. But again, I don't know of one.

And as you noted above, if we don't suppress the helper calls inside
Git, then every matching storage helper needs to learn about "nocache"
(or "ttl") before it will do any good.

-Peff