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

[Jeff King <peff@peff.net>]

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.

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.

     And I can't think of any other cases whose security implications
     aren't similarly unlikely. But I haven't carefully gone down the
     list (and as I said, I'd be hesitant to support a blacklist until
     _somebody_ takes the time to do so).

> If we are doing any filtering, however, it is always hard, if not
> impossible, to take away what we originally granted, even by
> mistake, for any reason, even for correctness or for security, in a
> later release.

Yep, agreed.

I am OK staying with a whitelist. But I think we should be fairly
lenient in whitelisting hierarchies that people have a use for, and
which do not violate (1) or (2) above.

> We probably could sidestep it by introducing an end-user
> configurable "whitelist" somewhere.

Ugh. Please no. I do not want to have to think about explaining to
somebody that they can accomplish what they want with submodules, but
only by pre-configuring their ~/.gitconfig to allow certain keys so that
they can pass the appropriate config on the command line.

-Peff