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

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Peff,

On Thu, 28 Apr 2016, Jeff King wrote:

> On Thu, Apr 28, 2016 at 12:28:21PM -0700, Junio C Hamano wrote:
> 
> > Jeff King <peff@peff.net> writes:
> > 
> > > It's definitely sufficient, it's just annoying if a user shows up
> > > every week and says "I want X.Y", and then somebody else shows up a
> > > week later and says "I want X.Z".
> > >
> > > Are we serving any purpose in vetting each one (and if so, what)?
> > 
> > Personally I do not think we would need to filter _anything_ if we can
> > tell that the user directly said
> > 
> > 	git -c var1=val1 -c var2=val2 $cmd ...
> > 
> > and "git $cmd" ended up needing to spawn another "git" subcommand,
> > possibly in some other repository (i.e. "$cmd" in this case is likely
> > to be "submodule", but in principle it does not have to be).  If the
> > user somehow gives variables like core.worktree that are inappropriate
> > to be applied across repositories, that's user's problem, i.e. "don't
> > do it then if it hurts".
> 
> Right, we are talking about that direct case here. And any time our
> filter heuristic lets something through, it is probably "if it hurts
> don't do it" as the worst case.

The more I think about it, I actually think that we do the user a *really*
great disservice by filtering the CONFIG_DATA_ENVIRONMENT. If I call

	git -c ... $cmd

and that configuration is *not* picked up, it is much worse than letting
users shoot themselves in their own feet by specifying config settings
that are *prone* to wreak havoc.

> So I think the only two cases worth filtering are:
> 
>   1. Ones where we _know_ that the config is nonsense to pass along,
>      _and_ where a user might conceivably make use of the
>      just-the-top-level version of it (core.worktree
>      comes to mind, though of course they are probably better served by
>      "--work-tree" in such a case).
>   2. An option where we think there may be some security implication.
>      Setting "http.sslverify" to false does have some security
>      implications ("oops, I only meant to turn off verification for the
>      root repo, and I got MiTM-attacked for the submodules!"). But it's
>      so obscure and unlikely that I think the benefit outweighs it.

I can see that happening when somebody calls an alias with `git -c ...`
and that alias performs actions in the top-level project as well as all
submodules.

But. Do we really have to be "Big Daddy" for users who do that?

> I am OK staying with a whitelist.

Me, too. But I am even more in favor of abandoning this "we know what is
good for you" approach, i.e. that whitelist that filters
CONFIG_DATA_ENVIRONMENT.

Ciao,
Dscho