Re: [PATCH v5 2/2] submodule: pass on http.extraheader config settings

> On Thu, Apr 28, 2016 at 02:19:37PM +0200, Johannes Schindelin wrote:
>
>> > Should we consider just white-listing all of "http.*"?
>> >
>> > That would help other cases which have come up, like:
>> >
>> >   http://thread.gmane.org/gmane.comp.version-control.git/264840
>> >
>> > which wants to turn off http.sslverify. That would mean it turns off for
>> > every submodule, too, but if you want to be choosy about your http
>> > variables, you should be using the "http.$URL.sslverify" form, to only
>> > affect specific servers (whether they are in submodules or not).
>>
>> I considered that, and thought that it might be dangerous, what with me
>> not vetting carefully which http.* variables are safe to pass on to the
>> submodules' update and which are not.
>>
>> So I had a look now, and the most prominent potential problem is the
>> http.cookieFile setting, which could be reused all of a sudden if we
>> made my patch more general.
>>
>> But then, we are talking about the code that filters what gets passed via
>> the *command-line*. And to be quite honest, I am not sure that we should
>> actually filter out *any* of these settings.
>
> The intent of the whitelist (from my recollection of the discussion) is
> to filter out config that must be repo-specific. E.g., core.worktree or
> core.bare should definitely _not_ be passed to a submodule.
>
> I don't know if there are others. We started with a whitelist because it
> was the smallest and safest change away from the status quo. A blacklist
> would also work, with the risk that we might let through nonsense in
> some cases (but only if the user triggers us to do so).
>
>> The commit message that introduced this particular filtering has this
>> rationale to let only credential.* through:
>>
>>     GIT_CONFIG_PARAMETERS is special, and we actually do want to
>>     preserve these settings. However, we do not want to preserve all
>>     configuration as many things should be left specific to the parent
>>     project.
>>
>>     Add a git submodule--helper function, sanitize-config, which shall be
>>     used to sanitize GIT_CONFIG_PARAMETERS, removing all key/value pairs
>>     except a small subset that are known to be safe and necessary.
>>
>> Dunno. I tried to err on the side of caution... But this sounds maybe a
>> bit *too* cautious?
>
> So if we all agree that the sanitizing is really about preventing
> repo-specific variables from leaking, and not any kind of security
> boundary, I think we should generally be pretty liberal in whitelisting
> things.
>
> I can certainly come up with a pathological case where using it as a
> security boundary may have some practical use, but in general I think it
> is mostly getting in the way of what users are trying to do.
>
> -Peff