Web lists-archives.com

Re: [PATCH 0/5] Multiple hook support




Hi,

brian m. carlson wrote:
> On Tue, Apr 23, 2019 at 07:34:38PM -0700, Jonathan Nieder wrote:
>> brian m. carlson wrote:

>>> I've talked with some people about this approach, and they've indicated
>>> they would prefer a configuration-based approach.
>>
>> I would, too, mostly because that reduces the problem of securing
>> hooks to securing configuration.  See
>> https://public-inbox.org/git/20171002234517.GV19555@xxxxxxxxxxxxxxxxxxxxxxxxx/
>> for more on this subject.
>
> I know this is a common issue, but fixing it is a non-goal for this
> series. Anything we do here is going to have to be backwards compatible,
> so we can't make any changes to the security model.

I think it's worth bringing up because we should have some idea of where
we want to head.

I think the backward compatibility part is actually one of the easier
aspects of this one.  We don't have to change the security model right
away because there are similar places to exploit like core.pager.  To
address that, we need a notion of configuration that individual repos
and worktrees can't override, and using such a configuration item, we
can provide a way to opt in to the new security model.  That provides
a smooth path forward.

[...]
>> Solving (1) without (2) feels like a bit of a missed opportunity to
>> me.  Ideally, what I would like is
>>
>>    i. A central registry of trustworthy Git hooks that can be upgraded
>>       using the system package manager to address (2).  Perhaps just
>>       git-hook-* commands on the $PATH.
>>
>>   ii. Instead of putting hooks in .git/hooks, put a list of hooks to
>>       run for each event in .git/config.
>
> The problem I had with this when discussing it was that our
> configuration system lacks a good way to control inheritance from outer
> files.

The standard approach in lists defined in Git configuration is for
assigning an empty item to clear / restart the list.  See
http.extraHeader for an example.

Thanks and hope that helps,
Jonathan