It seems I sent my original reply only to the github PR. Sorry for the confusion: On Mon, Oct 11, 2021 at 1:39 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > "Ivan Frade via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > > > From: Ivan Frade <ifrade@xxxxxxxxxx> ... > > It of course is a different matter if the explained idea is > agreeable, though ;-). Hiding the entire packet, based on the "it > might be in some setups" seems a bit too much. > > Is it often the case that the whole URI is sensitive, or perhaps > leading "<scheme>://<host>/pack-<abc>.pack" part is not sensitive at > all, and what follows after that "public" part has some "nonce" > material that makes it sensitive? In the specific case I am working on, the path of the URL is an encrypted string that shouldn't be completely exposed (exposing part of it would be fine). In general, I think we can assume that <scheme>://<host>/ are always "public" but the path could be sensitive. We could redact only the path (<scheme>://<host>/REDACTED), or even a fixed length of the URL? (<scheme>://<host>/pack-<xxREDACTED). In the next patch version I go with redacting the path. > > Changes since v1: ... > Please write such material below the three-dash line. Done > And there is no need to duplicate the log message here ;-) Done > So "original_options" is used to save away the reader->options so > that it can be restored before returning to our caller? > > OK (it may be more common in this codebase to call such a variable > "saved_X", though). In the latest iteration, the option is enabled for all sections and there is no need to set/unset the flag. > > + grep "clone< <redacted>" log > > This checks only that "redacted" string appears, but what the theme > of the change really cares about is different, no? You want to > ensure that no sensitive substring of the URI appears in the log. > > Imagine somebody breaking the redact logic by making it prepend that > string to the payload, instead of replacing the payload with that > string---this test will not catch such a regression. Now the tests verify the expected packfile-uri full line is in the log. Thanks, Ivan
