Re: [PATCH v2 0/3] Safer GIT_CURL_VERBOSE

[Jeff King <peff@peff.net>]

On Wed, May 13, 2020 at 12:33:37PM -0700, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Jonathan Tan <jonathantanmy@google.com> writes:
> >
> >> Thanks everyone. I went ahead with GIT_REDACT_AUTHORIZATION to match
> >> GIT_REDACT_COOKIES, with the default being true (i.e. you need to set it
> >> to "0" to have behavior change).
> >
> > Hmm, I would actually have expected us to move in the direction of
> > deprecating specific REDACT_BLAH and consolidate them into one,
> > instead of adding another one.  Especially as the primary reason why
> > we redact cookies is to protect those that are used for auth anyway.
> 
> Also I had forgot to grep for anonymi.e to find transport_anonymize_url(),
> which I was hoping that the new environment variable would cover to help
> those who debug.

Yeah, I'd agree with both of these. If Jonathan doesn't feel like
working on transport_anonymize_url() now, I don't mind if we leave that
for later. But let's come up with a general scheme that we're aiming
for, so we minimize changes to things that users are exposed to.

IMHO an option that only impacts the format of the human-readable trace
output is not something we need to deprecate. It's an internal detail,
and people can't rely on the exact format of the trace anyway. So I'd be
fine to just kill off GIT_REDACT_COOKIES completely (especially if the
default behavior is the safer "always redact").

That said, a boolean GIT_REDACT doesn't quite do the same thing, because
it's actually a list of cookies. My gut feeling is that this is a bit
over-engineered. Any cookies that Git uses are likely to be sensitive,
so just treating their values like auth (redacting by default, but
allowing them to be unblinded when the user asks for it). But maybe
Jonathan had a specific tracing case in mind, as the author of the
original.

-Peff