Jeff King <peff@xxxxxxxx> 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?
