Re: [PATCH v2] hooks: propose project configured hooks

On 2021-04-05 at 22:45:10, Albert Cui wrote:

I think part of the problem is that an automated process to update hooks is generally a security vulnerability, since it means that untrusted remote code will automatically run on your computer.

I want to be clear that I understand the desire for this feature, even though it's not a feature I would personally use, and the fact that there are many approaches means that clearly there are many people that do want this functionality. I have in the past shared hooks with others and we have mutually benefitted enormously from that fact. My concerns here are solely about the security aspects of this feature.

I don't think this materially improves security. All of these options have the same security problems, and that's inherent in the solution. What we're doing here is basically giving people a built-in feature that is the equivalent of piping curl to bash and blessing it as secure when it's not.

I agree that Git tries to be careful about security. It is for these reasons that I think Derrick and I have provided you the feedback we have here.

I think you've hit the nail on the head here, but drawn a mistaken conclusion. The average user doesn't have the ability to review hooks in general and therefore cannot make an informed decision about whether to enable them, so the behavior we need to have is not to lead them to doing things which are risky from a security perspective.

If my goal is to just build a product and not to run its tests, which I do with a decent number of projects, then I can audit a Go or Rust project trivially and determine if it executes arbitrary code or not during the build process and if so, inspect it and gain confidence in it. In fact, there are many projects which don't execute build scripts during the process, and therefore which are completely safe. This hook design changes that calculus dramatically.

I also want to point out that people clone repositories for a variety of reasons. At GitHub, every team has its own repository with documentation. Literally every employee at the company, regardless of role, interacts with a Git repository, even people who do normally nontechnical tasks such as our in-house lawyers and our event planners. Many of these people are nontechnical, and almost none of these repositories has any software development involvement. There are also numerous people elsewhere who may work on projects such as books or other non-software in repositories who are nontechnical. Under the current model, the biggest problem these people face is accidentally corrupting their local repository and losing data. With a design that prompts them to install hooks, they face the possibility of arbitrary code execution.

The reason I proposed the FAQ we have in our documentation is because I answer a decent number of questions on Stack Overflow, in addition to questions that involve users that I get pulled into at work. Overwhelmingly, the vast majority of users, even developers, are not completely comfortable with Git and are unsure about how to use it effectively (cf. https://ohshitgit.com/). If we propose to a user that they should do something like enable hooks by adding a prompt, many users will automatically say "yes" because (a) they don't understand and they trust that Git is prompting them to do something beneficial and (b) because they don't know or care and just want to get on with their lives. As a result, we're exposing people to giant social engineering attacks on behalf of potentially unscrupulous repository maintainers.

This is made worse by the fact that we will prompt users even when cloning a repo that they have no intention of performing development on means that we will have users who are misled here where otherwise nothing would happen.

There is a huge problem with social engineering attacks and phishing on the Internet today and I'm concerned that this is going in exactly the wrong direction.

I would want to see a comprehensive security analysis feature taking into consideration social engineering attacks, the skill level and comfort with Git of the majority of Git users, and the fact that people clone repositories for many reasons other than software development. It's easy to look at this from the perspective of the typical employee at a major tech company and assume that users are generally security conscious, comfortable with Git, and primarily engaged in software development on the projects they clone, but I'm not sure any of those cases are generally true, and anyway there are many counterexamples in the real world whose use cases we need to take into account.

I continue to have serious reservations about this series and approach, and I'm not sure that any proposal we can adopt here will address the security concerns. To be frank, I don't think this proposal should move forward in its current state or otherwise, since I think the security problems are inherent in this approach and fundamentally can't be fixed.

This is, as should be obvious from my email address, my personal opinion, despite my reference to my employer above. Unless otherwise stated, I don't speak for my employer and they don't speak for me.