Re: [PATCH v2 5/8] hook(clone protections): add escape hatch

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> In either case, we're considering config to be a trusted source of
> truth, so I think the security properties are the same. But for the
> system here, a user updating a hook needs to do multiple steps:
>
>   - compute the sha256 of the hook (for which we provide no tooling
>     support, though hopefully it is obvious how to use other tools)
>
>   - add the config for the sha256
>
>   - install the new hook into $GIT_DIR/hooks

I am not sure why any of the above is needed.  

Hmph.

I was somehow (because that is how "git config --help" explains
"safe.hook.*") led to believe that this "safety" was only about "git
clone would prefer not to run ANY hook before it finishes operation
and gives back the control to the end user, but historically it ran
any enabled hooks in the resulting repository that was freshly
created by it---so let's at least make sure the contents of the
hooks are known-to-be-good ones when 'git clone' runs the hooks".
Most importantly, once "git clone" gives control back to the end
user and the end user had a chance to inspect the resulting
repository, the files in $GIT_DIR/hooks can be updated and the hooks
will run without incurring any cost of checking.

Isn't that what happens?

Looking at the control flow, hook.c:find_hook() is the one that
calls the function is_hook_safe_during_clone() to reject "unsafe"
ones (and allow the white-listed ones), but I do not know offhand
how the code limits the rejection only during clone.  So perhaps
this set of patches need further work to restrict the checks only to
"while we are cloning" case?