On Thu, Apr 28, 2016 at 2:00 PM, Jeff King <peff@xxxxxxxx> wrote:
> On Thu, Apr 28, 2016 at 12:28:21PM -0700, Junio C Hamano wrote:
>
>> Jeff King <peff@xxxxxxxx> 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).
My gut reaction to this:
In this specific case I would rather error out, as you never want to have
core.worktree to point at the same dir for all of the repo and submodules.
Thinking about it further, I am not so sure any more.
(What if you have multiple submodules tracking the same project
and you want to see each submodule version with the one worktree you point to?
Highly unlikely edge case, but it voids the /never/ assumption of my
gut reaction)
>
> 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).
I think in this case, it is more likely for the user to say:
I know my ssl is so borked, so I want all traffic with no ssl
(including submodules).
>
>> 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.
I view the whitelist more like an "emergency knob to turn, because the
developers did it wrong and I want it now". the general case should be
covered by a mechanism we provide?
Thanks,
Stefan
>
> -Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html