Re: [PATCH] http: redact curl h2h3 headers in info

From: Glen Choo <chooglen@google.com>
Date: Nov 10, 2022, 17:48 UTC
Message-ID: <kl6lpmdu3dl5.fsf@chooglen-macbookpro.roam.corp.google.com>
In-Reply-To: <Y2xnUyEoCFmbPtUI@nand.local>

Taylor Blau <me@ttaylorr.com> writes:

Show 6 quoted lines

>>      * How could we set up end-to-end tests to ensure that we're testing
>>        this against affected versions of curl? To avoid regressions, I'd
>>        also prefer to test against future versions of curl too.
>
> Does that necessarily matter? We want to make sure that we don't see
> sensitive headers from the h2h3 module with any version of cURL, no?

It would help, but it might not be worth setting up infrastructure for just this use case alone. Given the various platforms running tests against the Git codebase, we probably get close to a representative sample of the population with enough time.

I think it would be more important to have tests against HTTP/2.0. If we did, we probably would have already caught this, e.g. t/t5551-http-fetch-smart.sh:'GIT_TRACE_CURL redacts auth details' and friends.

Show 5 quoted lines

>> +		if (!redact_sensitive_header(&inner)) {
>> +			strbuf_setlen(header, strlen("h2h3 ["));
>> +			strbuf_addbuf(header, &inner);
>
> This leaks inner.buf, no?

Ah, you're right. Thanks.