hooks: propose repository owner configured hooks

From: Albert Cui <[email protected]>

Hooks today are configured at the repository level, making it difficult to share hooks across repositories. Configuration-based hook management, by moving hooks configuration to the config, makes this much easier. However, there is still no good way for project maintainers to encourage or enforce adoption of specific hook commands on specific hook events in a repository. As such, there are many tools that provide this functionality on top of Git.

We propose adding native Git functionality to allow project maintainers to specify hooks that a user ought to install and utilize in their development workflows. This patch documents the requirements we propose for this feature as well as a design sketch for implementation.

---
hooks: propose repository owner configured hooks

Hooks today are configured at the repository level, making it difficult to share hooks across repositories. Configuration-based hook management, by moving hooks configuration to the config, makes this much easier. However, there is still no good way for project maintainers to encourage or enforce adoption of specific hook commands on specific hook events in a repository. As such, there are many tools that provide this functionality on top of Git.

We propose adding native Git functionality to allow project maintainers to specify hooks that a user ought to install and utilize in their development workflows. This patch documents the requirements we propose for this feature as well as a design sketch for implementation.

[email protected]

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-908%2Falbertcui%2Fhooks-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-908/albertcui/hooks-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/908

 Documentation/Makefile                        |   1 +
 .../technical/repository-owner-hooks.txt      | 294 ++++++++++++++++++
 2 files changed, 295 insertions(+)
 create mode 100644 Documentation/technical/repository-owner-hooks.txt

index 81d1bf7a049b..cdd52143f883 100644
@@ -96,6 +96,7 @@ TECH_DOCS += technical/protocol-common
 TECH_DOCS += technical/protocol-v2
 TECH_DOCS += technical/racy-git
 TECH_DOCS += technical/reftable
+TECH_DOCS += technical/repository-owner-hooks
 TECH_DOCS += technical/send-pack-pipeline
 TECH_DOCS += technical/shallow
 TECH_DOCS += technical/signature-format

base-commit: a5828ae6b52137b913b978e16cd2334482eb4c1f

gitgitgadget

A copy of Android or Chromium project I have on my disk is owned by me in the distributed world---open projects do not and do not have to care who makes and has copies, even though they may care who can push to the project's own repository.

So avoid "repository owner configured", when you mean "project configured" or "project controlled".

On the other side of the coin is that this document should avoid reference to a "repository" in an ambiguous way, as it can refer to the central meeting place the project controls, lets developers to clone and fetch from, and push into, and it can also refer to the copy of that central meeting place individual contributors work in.

In our own documentation, we often refer to the former as "the central repository", and the latter as "a clone" (as in "you start working in your own clone").

No need to duplicate this text twice.

Ack, thanks for the correct terminology.

Good point

There are two opposite intentions here and the slash is making it confusing. Some projects have internal and external central repositories, and they want to encourage developers to push to the external repositories when possible.

I don't think there's a contradiction here. Users should always be able to decline hooks since it's their own machine, so the goal shouldn't be to prevent uses from declining setup.

That said, most users want to do the right thing so that their patches get accepted
e.g. adopt the right code styles via linting, so the goal should be to make it as easy
as possible for them to do that.

In the ideal case: automation detection is great and there are few false positives :)

If not, we can't break unattended clones, so the default clone behavior would always have
to be no-hooks-setup... maybe that speaks to git clone --recommended-setup?

+1 we'd need a method for a user to trigger a set up if they change their minds or
need to recover.

I don't think this is contradicting since the user is consenting, but maybe the principle
can be clearer:

"Execution of code must require user consent, and users should clearly understand
what they are consenting to." That is, I would imagine for a prompt for "always" we'd

1) have clear help text saying that hooks from $REMOTE will be automatically installed
when they change
2) give an FYI if hooks the hooks do change

I like this goal. It's something I proposed (off-list) before and did a small write-up of here:
https://lore.kernel.org/git/[email protected]/

As far as I recall the response in the room at the time was the expected combination of "sure that would be neat" and "let's see the patches". I.e. I don't think there's hard objections to the existence of such a facility, but of course the devel is in the details, and obviously I never followed-up with actually trying to implement it.

With config-based hooks this'll be much easier for the hook case, and I've tried to help that along[A]. I'd be interested in reviewing any effort in this area as well. The ".githooks/*" case in that proposal goes away with config-based hooks (since they wouldn't be special anymore, just config).

Thanks for the context and the support. Agreed, I think this is a natural evolution of config-based hooks.

[...]

In [0], I proposed only allowing pre-clone opt-in to suggested hooks if allowlisted in the config for the given remote, which seems similar to your previous proposal. Extending this idea to any config settings seems very reasonable. I'd love other people's thoughts about this.

It seems much safer to do this for a given remote, opposed to a local file path, no?

Mentioned in [0], the primary motivation for a magic config branch that lived outside of the worktree was "since the configuration is separate from the code base, it allows you to go back in history or to older branches while preserving "improvements" to the hooks configuration e.g. maybe the project has shifted to using a newer version of a linter."

[...]

Summarizing from [0]: Based on this feedback, I'm hearing "we should have a design for suggested configuration in general," but I don't think that necessitates that we should pursue generic configuration before hooks configuration.

To clarify, are you saying, today, git's config variables are pulled from the documentation? I.e. the documentation is the source of truth for what config variables are supported? o.0

To clarify, this proposal is just to set the hooks config that config-based hooks enabled e.g. running git config --add hook.pre-commit.command pylint on behalf of the user, so I'm not sure what "arbitrary data" you're referring to. At least, any problems I'd think we'd already address with config-based hooks.

This seems like an OK alternative to allow-listing based on remote, but are you expecting users to add a GPG key to their .gitconfig? Remote URLs seem much more user friendly (think IP address vs URL).

[0]: https://lore.kernel.org/git/CAMbkP-S-9cccMpU4HG0Wurqap-WkTmD2zk50nKd9kJ_oWO__qw@mail.gmail.com/

Not replying to everything here for lack of time, might need to loop back to this, please headsup me if there's outstanding things you'd like

Yes, probably. I don't think I thought much about it at the time.

It also means that your hooks will forever need to be aware of the union of all revisions in the project to work properly, or more likely they'll simply break on older revisions as e.g. a hook that needs to handle a test directory handles just "t", but it used to be called "tests".

It's also just un-git-y in *requiring* a remote. A .mailmap, .gitattributes etc. all work with a repo you find on-disk, why does config & hooks need to be different?

How would a user of such a repo suggest changes to a hook? Now it's fairly easy for e.g. .gitattributes, you change it, push a branch, ask for it to be merged etc.

If you want the same hook for all revisions ever having some light logic in the hook itself to check/cache that (it's executing arbitrary code after all) seems like a saner thing for those who have this "magic branch" use-case than to make it the default.

It's the principle that if we think you might stab your eye accidentally, maybe we should start with a plastic fork instead of going directly to a lightsaber :)

I.e. hooks are a *lot* more dangerous to get wrong. I think this sort of proposal would be better received if it started with a small whitelist of our least dangerous (but still useful to set in-repo .gitconfig) config variables. Then the worst thing that'll happen is you'll confuse git-diff a bit, not an RCE.

Yes, see generate-configlist.sh.

But we don't hard round-trip it, and we couldn't have an exhaustive list anyway due to things like userdiff, sendemail.identity etc.

But I'm mentioning it because the next logical step from there is to for some/most pre-declare them, and at that point it could be some struct that lists the danger level of each one, and from there whitelisting them for in-repo .gitconfig would be a logical next step...

As noted by brian m. carlson etc. in the side-thread in <[email protected]> the danger is that by making this a supported feature git becomes the social-engineering vector to fool users into trusting a command like that which they otherwise might not have.

That instead of saying you trust https://github.com/git/git your primary means of interaction with this feature would be saying you, as an example, trust Junio's GPG key.

And if you clone a git.git from *anywhere* and have already opted-in to whatever scary config we'd expose for tihs we'll trust and setup the hooks, if and only if they're identical to hooks we can find that were last signed by the trusted GPG key.

Git is content-addressable, something that caters directly to that is more consistent and user friendly than something that makes URLs magical as a model for trust.

If I want a trusted kernel I don't really care what URL I clone linux.git from, I care that I can resolve the content to something Linus signed.

So if I trust Junio's key and would like this to Just Work it would be better if I clone a mirror of git.git that it works, and I don't have to maintain a list of all mirrors myself. Trusting based on content and GPG keys gives you that, magic URLs don't.

It seems to me that in the BigCorp use-case this would also be useful. Presumably you just want blessed reviewed hooks to execute arbitrary code on your laptop, but if your co-worker forks a repo and has control of your magic branch you don't want to setup the potential harmful RCE. They might have malicious hooks.

But that's also annoying because in your BigCorp repo you'll presumably want the hooks to check/validate/whatever. Just because you cloned a fork you'd still like that.

If you trust (signed) content instead of URLs you can have your cake and eat it too. As long as your co-worker didn't modify the hook config in his fork it'll Just Work.

Yes, but I think that's better than hooks not working when we're operating on past commits for whatever reason.

Why is a remote required? The purpose of this proposal is for remotes to suggest hooks, but that doesn't mean that the existing hooks mechanism will no longer work.

It depends on the exact implementation, but in my suggestion [1], it is a patch or a PR on a branch.

[1] https://lore.kernel.org/git/[email protected]/

If hooks are per-version, this doesn't work for versions before when the hook was introduced.

[skipping discussion about general remote-suggested config]

This danger is being discussed there, and we're trying to balance this danger against the fact that projects need or want functionality like this (as evidenced by the available tools described in the original email).

I think this feature can be extended to trusting GPG keys later. Do you think that we should move to a model of trusting a key *instead of* (not "in addition to") a URL?

[snip until summary paragraph]

This seems like scope creep to me - perhaps a GPG key is more flexible than a URL, but I don't think we need this flexibility yet (and we can always add it later).

On Mon, Jun 21 2021, Jonathan Tan wrote:

[Most of this I've replied to in
https://lore.kernel.org/git/[email protected]/; or that's a better jump-off point to discuss, just replying to leftover bits here]

As a general matter because Git is a distributed VCS, so we should think about how new proposed features can integrate properly with distributed models of development.

If we make distributed development a second-class feature set we're responsible from e.g. guiding open source projects away from distributed repos and patch application etc. to centralized hosting.

We are also talking about exposing users to some manner of "do you trust this thingy?" UI from day 1.

Given the points about training users to accept insecure patterns from others on this topic in past discussions I don't we can just say we'll worry about the distributed aspect later, as that means e.g. someone who interacts with N forks of a repo in GitHub being encouraged to add N trusted remotes if they want their linting of PRs to work properly.

I think we should trust content, GPG keys are one way of getting there. But even a magic URL implementation can be based on trusting advertised content with what I'd think are fairly small changes. E.g. you'd clone a branch, it has hooks, you'd diff the relevant thing against your trusted remote's version, it's the same it passes.

From: Albert Cui <[email protected]>

Hooks today are configured at the repository level, making it difficult to share hooks across repositories. Configuration-based hook management, by moving hooks configuration to the config, makes this much easier. However, there is still no good way for project maintainers to encourage or enforce adoption of specific hook commands on specific hook events in a repository. As such, there are many tools that provide this functionality on top of Git.

This patch documents the requirements we propose for this feature as well as a design sketch for implementation.

---
[RFC] hooks: propose project configured hooks

V2:

* Clarify usage of the word repository; "repository owner" is specifically confusing so changed to "project"
* Add section about local vs remote hooks, and why think why there's still use case for local hooks
* Change design principles section to be more explicit about security considerations
* Introduce requirement for a command to set up hooks
* Add UX examples

[email protected]

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-908%2Falbertcui%2Fhooks-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-908/albertcui/hooks-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/908

Range-diff vs v1:

1: 2a558d06245d ! 1: 473e14edd441 hooks: propose repository owner configured hooks
@@ Metadata
Author: Albert Cui <[email protected]>

## Commit message ##
- hooks: propose repository owner configured hooks
+ hooks: propose project configured hooks

Hooks today are configured at the repository level, making it difficult to share hooks across repositories. Configuration-based hook management, by
@@ Commit message
adoption of specific hook commands on specific hook events in a repository. As such, there are many tools that provide this functionality on top of Git.

- We propose adding native Git functionality to allow project maintainers to
- specify hooks that a user ought to install and utilize in their development
- workflows. This patch documents the requirements we propose for this feature
- as well as a design sketch for implementation.
+ This patch documents the requirements we propose for this feature as well as
+ a design sketch for implementation.

+ Change-Id: I5f6747524b97c51dfe5fa28e48ea03981b2da5b8

## Documentation/Makefile ##
@@ Documentation/Makefile: TECH_DOCS += technical/protocol-common
TECH_DOCS += technical/protocol-v2
TECH_DOCS += technical/racy-git
TECH_DOCS += technical/reftable
-+TECH_DOCS += technical/repository-owner-hooks ++TECH_DOCS += technical/project-configured-hooks
TECH_DOCS += technical/send-pack-pipeline
TECH_DOCS += technical/shallow
TECH_DOCS += technical/signature-format

- ## Documentation/technical/repository-owner-hooks.txt (new) ##
+ ## Documentation/technical/project-configured-hooks.txt (new) ##
@@
-+Repository Owner Hooks Administration
-+-------------------------------------
++Project Configured Hooks
++------------------------
+
+Background
+~~~~~~~~~~
@@ Documentation/technical/repository-owner-hooks.txt (new)
+execute commands or scripts when certain Git events occur. Some use cases
+include:
+
-+* The pre-commit hook event: before committing, a developer may want to lint
-+their changes to enforce certain code style and quality. If there are style
-+issues, the developer may want the commit to fail so that they can fix the
-+issues.
++* The pre-commit hook event:
+
-+* The commit-msg hook event: after committing, repository owners may want to
-+enforce a certain commit message style. This may be out of necessity:
++ ** A developer may want to lint their changes to enforce certain code style
++ and quality. The project may want the commit to fail so that commits that
++ violate the project's style don't get uploaded.
++
++ ** The project may want to prevent developers from committing passwords or
++ other sensitive information e.g. via
++ https://github.com/awslabs/git-secrets[git-secrets].
++
++* The commit-msg hook event: the project may want to enforce a certain commit
++message style. This may be out of necessity:
+https://www.gerritcodereview.com/[Gerrit Code Review], for example, requires
+each commit to have a Change-Id in the footer.
+
-+* The pre-push hook: before pushing, repository owners may want to verify that
-+pushes are going to the correct remote to prevent leaks.
++* The pre-push hook: the project may want to verify that pushes are going to
++the correct central repository to prevent leaks.
+
+A common thread between these use cases is to enforce certain behavior across
+many developers working in the same code base, encouraging best practices and
+healthy code quality.
+
-+Hooks today are configured at the repository level, making it difficult to share
-+hooks across repositories.
++Hooks today are configured individually in each clone, making it difficult for
++project maintainers to enforce hooks usage across them.
+https://lore.kernel.org/git/[email protected][Configuration-based
-+hook management], by moving hooks configuration to the config, makes this much
-+easier. However, there is still no good way for project maintainers to encourage
++hook management], by moving hooks to the config, makes this easier; individuals
++can at least configure hooks to be used across multiple workspaces on their
++host. However, there is still no good way for project maintainers to encourage
+or enforce adoption of specific hook commands on specific hook events in a
-+repository. As such, there are many tools that provide this functionality on top
-+of Git (see <<prior-art, Prior Art>>).
++clone. As such, there are many tools that provide this functionality on top of
++Git (see <<prior-art, Prior Art>>).
+
+We propose adding native Git functionality to allow project maintainers to
+specify hooks that a user ought to install and utilize in their development
+workflows.
+
++Server-side vs Local Checks
++^^^^^^^^^^^^^^^^^^^^^^^^^^^
++
++A large motivation for this change is providing projects a method to enable
++local checks of code e.g. linting. As documented in
++https://git-scm.com/docs/gitfaq#_hooks[gitfaq], in some cases, server-side
++checks may be more appropriate, especially since developers can always skip
++local hook execution. We think there are still benefits to local checks:
++
++* Ensuring commit message style and preventing the committing of sensitive data,
++as described above. In particular, if CI results are public, as with many open
++source projects, server side checks are useless for guarding against leaking
++sensitive data.
++
++* Helps developers catch issues earlier: typically developers need to push to
++the remote to trigger server-side checks. Local hooks can be run anytime the
++developer wants. This is especially useful if the project has slow
++server-checks; catching issues locally can save the developer a lot of time
++waiting for CI. They are also useful for locally reproducing an issue identified
++in CI, helping resolve issues faster.
++
++* Since the typical goal of developers to submit changes to a central
++repository, their interests are aligned with the project maintainers'; while
++they can choose to skip local hook execution, it is in their interest to allow
++hooks to run at least before proposing code for submission to the central
++repository to increase the chances of the code getting accepted.
++
++In the ideal world, developers and project maintainers use both local and server
++side checks in their workflow. However, for many smaller projects, this may not
++be possible: CI may be too expensive to run or configure. The number of local
++solutions to this use case speaks to this need (see <<prior-art, Prior Art>>).
++Bringing this natively to Git can give all these developers a well-supported,
++secure implementation opposed to the fragmentation we see today.
++
+User Goals / Critical User Journeys
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
-+* As a repository owner / project maintainer,
++* As a project maintainer,
+
+ ** I want to enforce code style so I require developers to use a
+ standardized linter.
+
-+ ** I want to prevent leaks / share code widely so I check that developers
-+ are uploading code to the right place.
++ ** I want to prevent sensitive data from getting checked in.
+
-+ ** I want this to just work for all the developers in my repository, without
++ ** I want to prevent leaks so I check that developers are uploading code to
++ the right private central repository. Conversely, I may want to encourage
++ more open source development and encourage developers to push to public
++ central repositories.
++
++ ** I want this to just work for all the developers in my project, without
+ needing to support them through configuration.
+
-+* As a developer developing in a repository,
++* As a developer developing in a local clone,
++
++ ** I want to set up my workspace.
++
++ ** I want control over what code runs on my machine.
++
++ ** I want to set up my own hooks, in addition to or in lieu of project
++ configured hooks.
++
++ ** I want to skip hooks from running (for various reasons).
++
++Security Considerations and Design Principles
++~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
-+ ** I want to set up my workspace
++We must balance the desire to make hooks setup easy for developers --- allowing
++them to get hooks set up with low friction, and hence increasing the probability
++of them adopting these hooks --- with protecting users from the security risks
++of arbitrary code execution on their hosts.
+
-+ ** I want control over what code runs on my machine
++To inform the design, we propose these design principles:
+
-+ ** I want to skip hooks from running (for various reasons)
++* User consent: Users must explicitly agree to hooks usage; no hooks should
++execute without consent, and users should re-consent if hooks update. Users can
++opt-out of hooks.
+
-+Design Principles
-+~~~~~~~~~~~~~~~~~
++* Trust comes from the central repository:
++ ** Most users don't have the time or expertise to properly audit every hook
++ and what it does. There must be trust between the user and the remote that the
++ code came from, and the Git project should ensure trust to the degree it can
++ e.g. enforce HTTPS for its integrity guarantees.
+
-+* *Make it easy:* Developers just want to get their work done. Introducing
-+friction is bad, especially when it prevents development from even starting e.g.
-+workspace set up.
++ ** Since developers will likely build their local clone in their development
++ process, at some point, arbitrary code from the repository will be executed.
++ In this sense, hooks _with user consent_ do not introduce a new attack surface.
+
-+* *Treat hooks as software, not configuration:* We take seriously the
-+responsibility that comes with causing arbitrary code to run on a user's
-+machine. Such code needs to come from a reputable source, be automatically
-+updated, and run with user consent.
++* Give users visibility: Git must allow users to make informed decisions. This
++means surfacing essential information to the user in a visible manner e.g. what
++remotes the hooks are coming from, whether the hooks have changed in the latest
++checkout.
+
+Feature Requirements
+~~~~~~~~~~~~~~~~~~~~
@@ Documentation/technical/repository-owner-hooks.txt (new)
+Minimum Feature Set
+^^^^^^^^^^^^^^^^^^^
+
-+* A repository owner can specify a configuration for what hook commands are
++* A repository can specify a configuration for what hook commands are
+enabled for what hook events
+
-+* A repository owner can specify, in this configuration, where the hook
++* A repository can specify, in this configuration, where the hook
+commands reside
+
+ ** This could be a path to a script/binary within the repository
@@ Documentation/technical/repository-owner-hooks.txt (new)
+ ** This could be a user installed command or script/binary that exists
+ outside of the repository and is present in $PATH
+
-+* Users must explicitly approve hook execution at least once (yes/no)
++* This configuration should only apply if it was received over HTTPS
+
-+ ** This could happen during setup or at execution time
++* A setup command for users to set up hooks
++
++ ** Hook setup could happen at clone time assuming the user has consented
++ e.g. if --setup-hooks is passed to git clone
++
++* Users must explicitly approve hooks at least once
++
++ ** Running the setup command should count as approval, including if the user
++ consented during the clone
+
+ ** When a hook command changes, a user should re-approve execution (note:
+ implementation should not interfere with requirement listed in “Fast
+ Follows")
+
-+ * Users do not need to run setup scripts to install hooks --- the setup flow
-+ happens automatically at clone time
-+
+* Automation is able to continue to use clone and other commands
+non-interactively
+
@@ Documentation/technical/repository-owner-hooks.txt (new)
+Implementation Exploration: Check "magic" branch for configs at fetch time
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
-+Example workflow
-+^^^^^^^^^^^^^^^^
++Example User Experience
++^^^^^^^^^^^^^^^^^^^^^^^
++
++===== Case 1: Consent through clone
++
++....
++$ git clone --setup-hooks
++...
++
++The following hooks were installed from remote origin ($ORIGIN_URL):
++
++pre-commit: git-secrets --pre_commit_hook
++pre-push: $GIT_ROOT/pre_push.sh
++....
++
++===== Case 2: Prompting after clone
++....
++$ git clone
++...
++
++Remote origin ($ORIGIN_URL) suggest installing the following hooks:
++
++pre-commit: git-secrets --pre_commit_hook
++pre-push: $GIT_ROOT/pre_push.sh
++
++# instead of prompting, we could give users commands to run instead
++# see case 3
++
++Do you wish to install them?
++1. Yes (this time)
++2. Yes (always from origin)
++3. No (not this time)
++4. No (never)
++....
++
++===== Case 3: Re-prompting when hooks change
++....
++$ git pull
++
++The following hooks were updated from remote origin ($ORIGIN_URL):
++
++pre-push: $GIT_ROOT/pre_push.sh
++
++If you wish to install them, run git hook setup origin.
++
++If you wish to always accept hooks from origin, run `git hook setup --always
++origin`. You should only do this if you trust code changes from origin.
++
++To always ignore hooks from origin, run git hook ignore origin.
++....
++
++===== Case 4: Nudging when hooks weren't installed
++....
++$ git commit
++advice: The repository owner has recommended a 'pre-commit' hook that was not run.
++To view it, run git show origin/refs/recommended-config:some-pre-commit. To install it, run git hook setup origin pre-commit ++
++Turn off this advice by setting config variable advice.missingHook to false."
++....
++
++
++Implementation Sketch
++^^^^^^^^^^^^^^^^^^^^^
+
+* Perform fetch as normal
+
@@ Documentation/technical/repository-owner-hooks.txt (new)
+* A way to perform general set up of a code base e.g. installing dependencies,
+any first-build related tasks
+
++* Sandboxing hook execution to provide higher levels of security.
++
+[[prior-art]]
+Prior Art
+~~~~~~~~~

 Documentation/Makefile                        |   1 +
 .../technical/project-configured-hooks.txt    | 426 ++++++++++++++++++
 2 files changed, 427 insertions(+)
 create mode 100644 Documentation/technical/project-configured-hooks.txt

index 81d1bf7a049b..06ba5945c428 100644
@@ -96,6 +96,7 @@ TECH_DOCS += technical/protocol-common
 TECH_DOCS += technical/protocol-v2
 TECH_DOCS += technical/racy-git
 TECH_DOCS += technical/reftable
+TECH_DOCS += technical/project-configured-hooks
 TECH_DOCS += technical/send-pack-pipeline
 TECH_DOCS += technical/shallow
 TECH_DOCS += technical/signature-format

base-commit: a5828ae6b52137b913b978e16cd2334482eb4c1f

gitgitgadget

Oops :)

I avoid this by setting gerrit.createChangeId = false in my global config and adding an alias:
alias.gerrit-commit = "-c gerrit.createChangeId=true commit"

            ^~~~

The point about solution fragmentation is a strong one and I wonder whether it's being emphasized enough. There is obviously a need, or else people wouldn't keep writing all these things in the Prior Art section :)

[snip]

Meaning, non-HTTPS fetches should just not update this special branch?

AIUI, this is proposed to be part of git hook, right?

I don't think it needs to be part of this doc but it'd be nice to also support installing just a subset, like:

  git hook setup pre-commit
  git hook setup --interactive

  $ git clone
  ....
  Remote `origin` ($ORIGIN_URL) suggest installing the following hooks:

  pre-commit: git-secrets --pre_commit_hook
  pre-push:  $GIT_ROOT/pre_push.sh

[snip]

I think this says: "Can we run a user hook in a container that only has access to the repo in question?"

It sounds like a complicated answer. I could see legitimate reasons to want wider access than just the container - for example, some hook-specific configuration that doesn't fit the Git config format, or even something like updating a stats file to keep a record of how many commits I made/pushed/whatever every day, stored in a central location for reference at performance review time :) But I also don't know alllll that much about containerization - I think there are ways to hand over access to other needed files like this, right?

But then, I also feel yucky thinking about Debian telling me that my Git install also needs me to install Docker... :)

Worth thinking about and discussing at a later date, I'd guess.

I wonder whether it's useful to mention (in mails, I guess, not in the checked in doc) why these are bad - do they duplicate work between each other? Are they engaging in bad practices when interfacing with Git? etc.?

It would be a lot of work to collect, so maybe it's not that useful..

Thanks for writing up v2 / mailing it, Albert.

- Emily

+1, I like phrasing it this way.

Yes, though I erroneously forgot to include SSH as well. I think the main issue is
person-in-the-middle type attacks.

Correct, I think git hook is a natural evolution. This is a nice to have that we can document.

It definitely aligns with the security principles to notify, even if they have OK'd updates.

Good question. If we consider this as an extension of config-based hooks, then I think
it's logical to still support hooks in the repo itself. In documentation, we might suggest
that people who want to use this feature store the hook in the magic branch for that
reason.

Sorry for being so late in reviewing this.

My first reaction is that this feature is suggesting multiple security vulnerabilities as core functionality. It also seems to be tied to niche projects (in number of projects, not necessarily the size of those projects).

I was recommended in conversation to think of this as a way to take existing ad-hoc behavior and standardize it with a "Git-blessed" solution. I'm not sure this proposal makes a strong enough case for why having a "configure-hooks.sh" script in the base of the repo is not enough. It simultaneously does not use existing precedents like .gitattributes or .gitignore as direction in using the worktree at HEAD as a mechanism for communicating details. I find using a separate ref for hooks to be a non-starter and the design should be rebuilt from scratch.

I also expect that a significant portion of users will see a message like "this repository needs hooks" and will just say "yes" to get rid of the prompt. There needs to be sufficient opportunity for users to inspect the hook configuration and avoid frustrated or distracted users from doing the wrong thing.

Server-side checks should always exist, so users who don't follow the project's guidelines using the recommended hooks will be blocked. The important thing is that there is an easy way for willing participants to install the correct hooks. This doesn't mean we should make it almost automatic.

Also, please proactively pursue a security review of the feature, including non-technical risks such as social engineering, forks, or other possible attacks. This idea seems so risky that I would be against accepting it unless a security expert has done a thorough review.

...

...

...

I appreciate the motivation in this document. However, the motivation doesn't really justify why this should be baked into Git itself, since a "configure-repo" script in the base of the repo would suffice to achieve that functionality.

The reason to put this in Git is to standardize this process so it is not different in each repository. It might be good to spend time justifying that angle.

Binaries will be tricky if you want users of multiple platforms to interact with your repository. And scripts can be slower than binaries.

How could someone build hooks from source using your workflow? Perhaps users are expected to locally compile the code before configuring the hooks?

Avoiding http:// and git:// makes sense. Why not SSH?

According to it's GitHub page, Husky [0] has 22M million monthly downloads and is used by 437K projects. Many projects, both big (e.g. Android [1], ChromeOS [2],
Angular [3], Webpack [4]) and small have use cases around hooks configuration, so I don't think it's fair to say this is only needed by niche projects.

Right, this entire proposal is trying to get to a "Git-blessed" solution, and I should make the need clearer. A few reasons for standardizing this come to mind:

1. Many existing "standard" solutions. A multitude of existing solutions for this use case speaks to the fact that a basic config script is not sufficient. I mentioned Husky above, but here are a few more; basically each popular programming language environment has a solution for this.

https://github.com/sds/overcommit - Ruby
https://github.com/pre-commit/pre-commit - Python
https://github.com/Arkweid/lefthook - Go
https://github.com/shibapm/Komondor - Swift
https://github.com/typicode/husky - Node

These solutions all handle the installation and updating of hooks. A "configure-hooks.sh" script doesn't handle hook updates, unless you go through the trouble yourself of implementing and maintaining that.

2. External dependencies. The existing solutions require users to either have those
toolchains already installed or an explicit installation step via an OS package manager, so there's a lot of friction for users, even when using these standard solutions.
This functionality shouldn't require external dependencies, and most certainly _how_ you
set up hooks shouldn't change depending on your coding environment. Fixing this is only
possible in a world where Git supports this functionality natively.

3. Improving security. As you mentioned, hooks are difficult to get right from a security
perspective, and standardizing on a single implementation allows us to give developers
a well-vetted solution with a better security model than what exists today. For example,
we're proposing making it very clear to users whenever there's a hook update. This isn't
something that existing solutions do.

I'll also say in general, the Git project is much more likely to get security right than smaller
projects, where oftentimes even popular projects end up unmaintained.

For the design feedback: what I'm hearing is that you'd prefer a design that keeps configuration in the worktree. I'll leave discussion about implementation to those on the list better suited :)

However, one thing that I should have mentioned in patch about the design proposal (as Emily Shaffer touched upon in her response) is that since the configuration is separate from the code base, it allows you to go back in history
or to older branches while preserving "improvements" to the hooks configuration e.g. maybe the project has shifted to using a newer version of a linter.

This functionality has pros and cons. Bringing improvements to checks can be useful
e.g. it's arguably better to bring older code up to newer code style. But it also makes
it less possible to replicate the exact state of older history. This is a problem that
server-side checks also have.

What I'm hearing here is that you don't like the interactive prompt, as you noted in the UX feedback below. Allowing users the chance to inspect the hook configuration makes sense to call out explicitly as part of the security model.

Agreed. We already did a security review internally at Google. The main feedback was:

* We need an explicit opt-in opposed to setting hooks up automatically, e.g. a command line flag like --accept-hooks at minimum. This is primarily to distinguish people who are just cloning a repository to browse the code from people who are developing.

* The average user doesn't have the ability to review hooks in general (security is hard and obscuration is easy), and if the user has already opted into
this feature because they are engaged in development, it's very likely that they're
already running build scripts, so the additional attack vector here doesn't seem like a big issue.

If there are other security folks I should seek advice from, please let me know.

[...]

I am happy to extend this patch to talk about configuration in general with a specific use case of suggesting hooks. However, I do want to separate out "what should we do when?" from "should we do this?" Based on this feedback, I'm hearing "we should have a design for suggested configuration in general," but I don't think that necessitates that we should implement, for example, partial
clone configuration before hooks configuration.

Sorry, this wasn't meant to come off as "small projects don't need CI." It was more to say, "small projects today are already primarily relying on local checks,
so we should make that easier for them." I clarified that a bit here as well:
https://lore.kernel.org/git/CAMbkP-Q4_z-nQzJwr2ZeM16deHKmzr=Z4908UzgOyk9FA-gKEA@mail.gmail.com/T/#u

[...]

Thank you for the feedback. I hope what I wrote above helps.

[...]

Responding to this below in the "Consent through clone" section.

Responding to this below.

"Binaries" was primarily referring to packages like pylint which are installed at the OS level. In an enterprise environment, these could be installed automatically
for the user.

If we notify users about changes, both to hook commands and to the underlying source files, we can make this safer.

As mentioned, for enterprise deployments, this can be solved by administrators installing any necessary software automatically.

Otherwise, I think ensuring the tool is installed feels out-of-scope (as written in
the doc); it's not like Git makes sure compilers or build tools are installed today,
and even today, users could set up Husky hooks that rely on $PATH tools, so we're not introducing a new problem.

Some day, maybe we could have a post-clone and post-checkout hooks, but I think this is all out-of-scope of the immediate discussion.

Left off SSH accidentally! Definitely makes sense to include.

I hear what you're saying. How would you feel if this specific functionality was behind
config (that enterprise environments could ship)? What I'm thinking:

```
#~/.gitconfig
[hook]
   allowCloneInstallFromRemote = $REMOTE
```

IFF $REMOTE matches the config, then git clone $REMOTE --setup-hooks works.

(We could even get rid of --setup-hooks and rename the config to hook.installOnCloneFromRemote but I think requiring user consent is still best here.)

As mentioned above, developers will execute code locally anyway when building. Do you think allowing users to opt-into hook updates significantly increases the attack
surface? I should note that existing solutions already do hook updates automatically and
silently (since they use a trampoline script); this proposal is safer from that given
1) we ask users to opt into auto updates and 2) we warn users about changes.

This should say "When prompted to install a hook, users can specify always or never, even if the hook updates. For security, this consent should be tied to a specific remote."

[trimming some duplicate points]

Sure, case 3 seems like a reasonable path to pursue. Sounds like auto-updating is the main point of contention here.

[...]

#2 is a feature, as mentioned above, but certainly just a "nice-to-have." I agree we'd
need a solution to #3.

Thanks for all the feedback!

Albert

[0] https://github.com/typicode/husky
[1] https://android.googlesource.com/platform/tools/repohooks/
[2] https://chromium.googlesource.com/chromiumos/docs/+/HEAD/contributing.md#Upload-changes
[3] https://github.com/angular/angular/tree/master/.husky
[4] https://github.com/webpack/webpack/tree/master/.husky

I am afraid that this compares apples and oranges.

I may "git clone" and try "make" to find out that I needed a special compiler, and that would not be the end of the world. It is guaranteed that "git clean -f -x -d" followed by installation of necessary toolchain followed by "make" would work. And that is partly because "git clone" does not do any more than just clone and checkout the initial tree.

If a new version of "git clone" told me "I can install the project recommended hooks to use", I answer "yes", and then failed while installing and configuring the project-recommended hooks because of missing dependencies, then I wouldn't know in what state the result would be in. In some projects, it may be enough to just install the missing dependencies, and in some others, it may not be enough and I have a broken half-configured mess depending on how the "installing and configuring" step failed.

I'm a little confused, and maybe it's because we have different definitions of what
"installing hooks" means. By installing hooks, I meant the addition of the hook command to the config, e.g the outcome of:

git config --add hook.pre-commit.command pylint

This works today; Git won't complain if I don't have pylint installed, so I don't see how we'd get into a "broken half-configured mess."

It will complain when it tries to execute the hook, and this is where I see it as the same as the 'I may "git clone" and try "make" to find out that I needed a special compiler, and that would not be the end of the world' case.

Yeah, I was envisioning that it won't be as a simple, mechanical and unconditional single command invocation. Rather, the 'pylint' part would have to become different depending on the environment (e.g. what operating system you are on, what editor you prefer, etc.). And the part that decides what kind of environment you are on would have to be written by the project that controls the "project-managed hooks"---unfortunately that would most likely to be an error-prone part.

I don't see a need for that. At least, this doesn't feel part of the "minimum feature set."

I don't think any existing solutions for hooks management handle this, so it feels fine to push this part to the project. As you said, if they wanted this, I'd expect git config --add hook.pre-commit.command lint.sh where lint.sh does whatever cross-platform magic is required.

I wouldn't expect a lot of need for this anyway; I'd expect most hooks to depend on tools that already work cross-platform (e.g. pylint) or be hooks written in the programming environment of the project, so the toolchain should already be in place to execute the hooks.

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.

brian m. carlson (he/him or they/them)
Houston, Texas, US

I agree with pretty much every word you said, in particular the social engineering aspect of this. In past mails I've referred to elsewhere I've proposed some Emacs-like "ask" facility for git, but you've convinced me that that default would be a bad idea for the "user just clicks yes no matter what" reasons you noted.

Still, I do think there is a way to make this work in a way that's probably acceptable for everyone:

* We don't ever ask the user to install hooks, it's something that they'd have to know about and pro-actively set up in advance.

* The security model is entirely focused on not approving changes as you "pull" them, but e.g. GPG-verifying the whole chain with some pre-setup key.

The use-case (and I've had this use-case in the past) would be something like a BigCorp which automates its servers/laptops, but would prefer not to patch/build/ship something like git itself.

So when you "git clone" your corporate repos you get relevant config/hooks, but not otherwise. We'd of course have a way to discover that you can set these up & do so after "clone", but it would be something more like check-mailmap, not something we'd prompt you to do.

I'm personally much more interested in doing something like this for an in-repo .gitconfig, with us shipping a graduals whitelist of known config values at differeng safety levels.

That sort of thing really *is* something we could imagine asking the user about, or even doing by default, e.g. applying a "diff -U<n>" setting for that repo, picking up non-executable aliases for non-data-changing git programs etc.

But as you note hooks are really on the extreme other side of that security curve, which is why in some earlier thread discussing this I suggested that a much more productive way to start an effort like this would be the in-repo .gitconfig route. We could start with our N most safe config variables, and work from there...

These replies definitely speak from a perspective common to mine. This is very dangerous territory and should be handled carefully.

There is also a legitimate user need to use hooks _to contribute_ to some repositories. Hooks are not needed to read the repositories or interact with them as a document.

The current mechanisms require ad-hoc approaches that are custom to each project, so there would be value in creating a standard inside the Git client itself. I think the proposal goes too far in making this an automatic configuration, either because it assumes trust or assumes sufficient skepticism on behalf of the users. Either is not acceptable for the Git project.

Here are the hard lines I draw:

1. This should not happen in "git clone" (other than maybe a message over stderr that hooks are available to be configured through a different command).

2. Hooks should not update in "git checkout" (other than a message that hooks have updated).

3. Whatever document triggers a hook configuration should live at HEAD and should not be configured or updated until HEAD has been updated by one Git command (git clone, git checkout), time passes for the user to inspect the worktree, then _another_ command (git hooks?) is run manually to reconfigure the hooks.

I think there is a potential way forward if these items are followed.

But I'd like to ask a different question: What problems are these custom hooks solving, and can Git solve those problems in-core?

If we care about checking commits for format or something, is that a common enough problem that we could implement it in Git itself and enable it through a Git config option? It might be interesting to pursue this direction and maybe we'll solve 80% of the need with extensions like that.

I'm aware of some hooks that insert things like a Gerrit change-id that would probably not be appropriate for such an in-core change.

There is always the extreme option of requiring users to use a specific fork of Git in order to work with your repository. That has its own pains, believe me. But, it does allow for the ultimate flexibility in how these things are done. Optional config can be enabled by default. Hooks can be replaced with in-core functionality.

Thanks,
-Stolee

To Ævar's point, maybe it would help to separate the two user bases of project configured hooks.
(1) Employee working at BigCorp. They are cloning from a trusted remote on company machines where the company controls what gets installed and how Git is configured. Their motivation is to make changes to their local clone and submit changes to a central repository.
(2) Git user cloning from any remote e.g. GitHub. They could have many
motivations: to make changes, to inspect the code, to simply just build.

I agree that this feature should not get in the way of users (2), or expose them to new attack surfaces, users who may have no desire to have project configured hooks. That said, I think we can still get into a world that better serves users (1). I proposed this upthread and would like feedback on it (I realize these examples still assume one config for every branch, but you get the gist):

Case 1. Opt-into clone setup via config

```
#~/.gitconfig
[hook]
   allowCloneInstallFromRemote = $REMOTE
```

IFF $REMOTE matches the config, then git clone $REMOTE --setup-hooks works:

```
$ git clone $remote --setup-hooks
The following hooks were installed from `origin` ($ORIGIN_URL):
pre-push: $GIT_ROOT/pre_push.sh
```

Case 2. Without the config opt-in for clone setup

```
$ git clone $remote # using --setup-hooks here wouldn't change
behavior since there's no config opt-in
Remote `origin` ($ORIGIN_URL) suggests the following hooks:
pre-push: $GIT_ROOT/pre_push.sh

If you wish to install them, run `git hook setup origin`.
To always ignore hooks from `origin`, run `git hook ignore origin`.
```

Case 3. Opting into updates
You could imagine a similar config, e.g. allowAutoUpdateFromRemote that allows Git to prompt users to consent to auto-updating hooks on "git checkout" with this type of behavior:

....

$ git checkout

The following hooks were updated from remote origin ($ORIGIN_URL):

pre-push: $GIT_ROOT/pre_push.sh

If you wish to install them, run git hook setup origin.

# The below only appears if allowAutoUpdateFromRemote is set for $ORIGIN_URL If you wish to always accept hooks from origin, run git hook setup --always origin. You should only do this if you trust code changes from origin.

To always ignore hooks from origin, run git hook ignore origin.
....

I want to separate the requirement from the implementation. What I'm hearing is that "users should have a chance to inspect the suggested hook before consenting to installing it." That doesn't necessarily require the configuration to be in HEAD.

Again, that's reasonable for users (2) but doesn't seem necessary for users (1) if we have the correct opt-ins.

A git lint command would cover a lot of the use cases, but to your point, there are others.

Thanks,
Albert

Hmph.

If the assumption is that their configuration is controlled by BigCorp when they arrive at their desk, why do you even need any change to upstream Git, especially with Emily's "configuration file tells Git what hook scripts to run" in sight?

Wouldn't it be just a matter of the BigCorp installing /etc/gitconfig on their BigCorpLinux installations?

FWIW I've been assuming that this is wanted for BigCorp people who have devs using vanilla OS's / XCode etc. Having managed laptops with a managed /etc/gitconfig certainly makes some things easier...

Whether you're at BigCorp or not you've got the problem with such a /etc/gitconfig that it applies to all repos, but someone at BigCorp might clone both a BigCorp repo (wants custom config, hooks etc.), and also some random node.js github.com project (should have no such custom config).

Having a .gitconfig or otherwise making the repo suggest/control the config/hooks is an elegant way around that issue.

You otherwise need to do something like have config includes apply a rule to ~/work/, and then hope your users follow your suggestion to clone repos in the ~/work/ folder.

I think extending config includes to e.g. operate on a glob of the remote URI was suggested at some point, which would make use-cases like that easier than they are now, and would be a less invasive change than what's being discussed here.

But right now we don't have that, so we have a gap there...

In particular, FreeBSD is an example of a platform that is not in the above list, but included in Git's CI. Is there an explicit list of supported platforms (and perhaps a notion of support tiers)?

I was looking for repository-provided local hook support for the FreeBSD repositories, and came across this proposal. I'll explain our desire, in case it can provide some insight. (We started migrating from Subversion to Git some time ago and finished the last repo a couple of weeks ago.)

We use one hook today, and would like developers to keep it up to
date: prepare-commit-msg. We have some standard commit message trailers like Reviewed by: and Differential Revision: that are provided as templates, and our prepare-commit-msg adds these to the default git-provided message that lists changed files and such. These trailers are updated occasionally - for example, we recently adopted "Fixes:" and added it to the template.

For us, I think a message that hooks are available or updated, and a command to install or update them would be fine. I can foresee some usability concern when switching branches - for example, to an older release branch, but it's probably not a large concern.

FreeBSD effectively did this when we used Subversion - the modified commit message template was provided by a patched Subversion via our ports collection. As you suggested it's workable, but not great.

It is not like there is a Git company who employs developers to support certain platforms. This is the mailing list for the open source development community for Git, and Developers come and leave over time [*].

The best you can get out of here is: if you find portability issues when you tried to make it work on your favorite platform, you can raise it here and (1) somebody with more Git experience on the same platform may solve it for you, (2) somebody with similar Git experience as you do (or less) on the same platform may solve it with you, or (3) somebody without acess to the platform may offer to help you solve it.

You cannot reasonably expect more than that from us on this list.

However, major platforms have their own (often binary) packaging system that offer Git packaged for their users. Debian/Ubuntu ship their own .deb, Git-for-Windows binary installer is made available promptly after we release a new version, macOS folks have Apple Git and homebrew or macPorts or whatever (but do not quote me on this; I am not an Apple user). What they do is not under our control, and you need to ask them what their support policies, EOL timelines, and such. Some of these packagers lurk around here and may respond, but that is you getting lucky ;-)

[Footnote]

* You can peek into config.mak.uname to see the list of platforms that have had a working Git some time in the past. Hopefully most of them are still up-to-date and working, but we wouldn't even know if a minority platform, for which an entry for it was added to the file in the past by some developer who needed a working Git on it, no longer works with the latest version of Git with recent toolchains after the original developer lost interest.

I'm sorry that my query wasn't clear; I have no expectation of Git volunteers providing support (in the commercial sense) for any particular platform.

What I am interested in is the Git community's expectations around platform support, with respect to new features, changes that break one or more platforms, and similar. I submitted portability improvements for FreeBSD, and certainly expected that if a change introduced a regression on one of Linux, Windows, or macOS it would not be accepted.

Yes - this is what I'm wondering about. It seems this information is not available other than by inspecting config.mak.uname or reading mailing list archives. I can also look at .travis.yml, .cirrus.yml, and .github/workflows/main.yml to get a sense of the platforms supported by Git's CI. That includes at least various versions or flavours of Linux, macOS, Windows, and FreeBSD. Is it worth putting a sentence or two in README.md about this?

I do not think that README, which is end-user facing, is a good place for that.

If anything, I am inclined to say that developer facing documents like SubmittingPatches and CodingGuidelines may be a better place to say that things are expected to work on X, Y and Z platforms, but I would think it is perfectly OK to start with a new feature that works only on, say, macOS, and leave it unimplemented for other platforms until somebody who is motivated enough to help fills the gap. So in that sense, "things are expected to work on these platforms" may be already stronger than what we want to say.

We don't have a fixed set of supported tiers like, e.g. Rust. We have CI for some platforms, and we have people who routinely run Git, including RCs and development branches like next, on various platforms and report back. If something breaks CI, obviously you are expected to fix it, and if someone says you broke their platform, you are expected to unbreak it (for open source systems like FreeBSD where you can spin up a VM) or at least work with the interested party to unbreak it.

Otherwise, support is best effort. While I don't use FreeBSD, I'm reasonably aware of what functionality it does and doesn't support, and I'll try to avoid inserting Linuxisms into our code. Similarly, sometimes people ask us to support some obsolete OS which doesn't have security support (e.g., CentOS 5), and sometimes we accept patches for that. (I am personally opposed to supporting systems without security support, but other developers feel differently.) We will generally accept reasonable portability patches for most OSes with little fanfare.

Developers often will CC maintainers of specific OSes (most often, Windows) if they want to make sure that the patches being proposed meet that platform's needs.

I have broken macOS in an edge case in the past due to its case-insensitive file system behavior, and nobody noticed until the release. Since our testsuite lacked a test for that case and nobody running macOS pre-releases on a regular basis hit that case (two files in the repository differing only in case) and complained, it got shipped broken, although we did promptly fix it.

That's the kind of support level we have. Basically, we do our best, and if there's a problem and someone shouts, we'll fix it.

brian m. carlson (he/him or they/them)
Houston, Texas, US

Formatting nit:

Yes, thanks. Good catch.

I had comments on the v1 that are still unanswered by this re-roll:
https://lore.kernel.org/git/[email protected]/

I think a more productive way to handle such proposals is to reply to such general discussion and /then/ re-roll the series.

I'm not going to repeat the outstanding points there (but would like you to reply to it, and having just read Derrick Stolee's E-Mail downthread there's another significant point of feedback to reply to.

So just comments on the range-diff below. I think for any one paragraph reading ahead helps, because it's a bit of stream-of-conciousness as I try to make sense of things that are then hinted at in later parts of the document (which itself is a suggestion to maybe re-arrange some of it).

Why does the project want to prevent developers from *committing* such information by default? I commit such stuff all the time, it's only a problem once it's pushed.

The genesis of this series seems to be need within the Google-plex. Having operated a similar central-repository corporate setup before I'm surprised that you're even trying to prevent such things on local computers, surely you'll have to re-do such checks on-push on the server, so just have them live there?

But I think this is answered below:

So what you really mean is you want a pre-commit hook as an alternative to some "once we've accpted the push and CI runs we notice naughty data", not as a pre-receive hook alternative?

This all makes sense, but is really missing how this problem isn't adequately solved by:

    $ HACKING
    Welcome to project XYZ, first you'll be much more productive with
    our hooks, run:

        ./setup-hooks.sh

    [...]

Presumably developers working on some central repo are the ones least served by this type of thing, since such setups usually have established setup scripts etc. that you (mostly) go through once.

The end-goal of this series combined with Emily's configurable hook is basically to have less friction when it comes to setting up and maintaining hooks.

I don't see how it would help with "CI may be too expensive to run or configure" though. We're basically talking about auto-updating a script in .git/hooks, which today is essentially a ./setup-hooks.sh, and the script checking for a new version of itself when it runs at origin/myscripts:myname.sh, no?

I think I'm beginning to get the gist of this, so it's really also a proposed workaround for projects that host on platforms like github.com and whatnot where you can run arbitrary code in a CI, but you can't install a custom pre-receive hook?

I think it might be worth explicitly spelling that out. E.g. if those platforms gained a feature (which isn't that hard to imagine) of hiding your ref until after some pre-receive part of a CI check has run (which would not have public logs, so you could "safely" check/push passwords) it seems to me that a large part of the immediate need for this would go away.

Which doesn't per-se mean we shouldn't do it, just that assumptions, constaints etc. should be documented.

I won't belabor the point but in my feedback on v1 I suggested an alternate mechanism of doing this which I think is more distributed-friendly and more safe:
https://lore.kernel.org/git/[email protected]/

...

That alternate security model I suggested above would make this much less painful. I.e. you could say "I'll trust updates as long as the whole chain is signed by XYZ authority".

Snip the rest of the doc, which as noted I've god unreplied-to feedback on in https://lore.kernel.org/git/[email protected]/

Thanks for the feedback! I'm new to this process. I'll make sure to reply to both.

[...]

I think you're identifying that "prevent developers from commiting" is the wrong wording.
Maybe a better phrasing is "prevent sensitive information from getting pushed or checked in."

Yes, this could be done in CI, but as noted below, catching this before the push (or commit)
happens is more productive for developers:

An additional point I should also call out is that for large repositories, pushes themselves
can be slow, so even if server-side checks are fast, being able to avoid unnecessary pushes
can help developer velocity.

One issue that immediately comes to mind with a setup script is that developers could
easily miss out on updates to the hooks. One nice thing about this proposal is that we
try to address that problem.

Anothing issue is that people in general are bad at reading instructions; that's why I'm
trying to get as close as possible to git clone doing set up for you while balancing the
security concerns (I know Derrick Stolee finds issue with this in his reply, which I'll try to
address in my reply there.)

By "expensive", I mean from both a money and a time perspective: for small projects, you may not set up any server-side checks and instead rely purely on local checks,
and this helps by, as you said, reducing the friction to set up these local checks.

From my own experience: When I start a new weekend project, setting up CI is not at the top
of the list of things I want to spend time on; I'm usually not even writing tests. Maybe
I'm just a bad developer :D But I do set up local checks: linting, code formatters.

To your point about updating hooks (we're thinking on the same lines!): that's putting the
responsibility on the developer to implement. From a maximizing-global-productivity
point of view, isn't it more elegant for us to extend Git's functionality and provide good support
for this use case?

Agree we can call this out, but as I'm saying above, I don't think it's fair to assume
everyone is already using server-side checks and there's still benefit to doing the same
checks locally even if you have server-side checks.

[...]

Really appreciate the engagement! I'll try to reply early next week.

From: Albert Cui <[email protected]>

Hooks today are configured at the repository level, making it difficult to share hooks across repositories. Configuration-based hook management, by moving hooks configuration to the config, makes this much easier. However, there is still no good way for project maintainers to encourage or enforce adoption of specific hook commands on specific hook events in a repository. As such, there are many tools that provide this functionality on top of Git.

This patch documents the requirements we propose for this feature as well as a design sketch for implementation.

---
[RFC] hooks: propose project configured hooks

V3:

* Add more motivation for feature
* Formalize security threat model
* Remove references to clone-time installation
* Update requirements for when advice should appear, config location
* Update UX examples
* Suggest auto-update hooks behind config

[email protected]

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-908%2Falbertcui%2Fhooks-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-908/albertcui/hooks-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/908

Range-diff vs v2:

1: 473e14edd441 ! 1: e8fbcc060c71 hooks: propose project configured hooks
@@ Commit message

- Change-Id: I5f6747524b97c51dfe5fa28e48ea03981b2da5b8
+ Helped-by: Jonathan Nieder <[email protected]>

## Documentation/Makefile ##
@@ Documentation/Makefile: TECH_DOCS += technical/protocol-common
@@ Documentation/technical/project-configured-hooks.txt (new)
+Project Configured Hooks
+------------------------
+
++// The '+' in Gerrit URL frightens asciidoctor.
++:chrome-os: https://chromium.googlesource.com/chromiumos/docs/+/HEAD/contributing.md#Upload-changes
++:repohook-read: https://android.googlesource.com/platform/tools/repohooks/+/refs/heads/master/rh/config.py
++:repohook-config: https://android.googlesource.com/platform/tools/repohooks/+/refs/heads/master#config-files
++
+Background
+~~~~~~~~~~
+
+Context
+^^^^^^^
+
-+Git has https://git-scm.com/docs/githooks[hooks] functionality to allow users to
++Git has linkgit:githooks[hooks] functionality to allow users to
+execute commands or scripts when certain Git events occur. Some use cases
+include:
+
@@ Documentation/technical/project-configured-hooks.txt (new)
+https://lore.kernel.org/git/[email protected][Configuration-based
+hook management], by moving hooks to the config, makes this easier; individuals
+can at least configure hooks to be used across multiple workspaces on their
-+host. However, there is still no good way for project maintainers to encourage
-+or enforce adoption of specific hook commands on specific hook events in a
-+clone. As such, there are many tools that provide this functionality on top of
-+Git (see <<prior-art, Prior Art>>).
++host.
++
++However, there is still no good way for project maintainers to encourage or
++enforce adoption of specific hook commands on specific hook events in a clone.
++As such, there are many tools that provide this functionality on top of Git (see
++<<prior-art, Prior Art>>) with a lot of adoption across projects big and small:
++
++ * According to it's GitHub page, Husky has 22M million monthly downloads and is
++used by 437K projects.
++
++ * Example open source projects that depend on this type of behavior include
++ https://android.googlesource.com/platform/tools/repohooks/[the Android Open
++ Source Project], {chrome-os}[ChromeOS],
++ https://github.com/angular/angular/tree/master/.husky[Angular], and
++ https://github.com/webpack/webpack/tree/master/.husky[Webpack].
++
++The wide adoption of solutions combined with the fragmentation in solution
++adoption speaks to the opportunity for the Git project to provide a standard
++solution to Git users. In doing so, we can provide a secure, well-supported,
++well-maintained, and better functioning solution (e.g. does not require extra
++dependencies) that benefits all Git users.
+
+We propose adding native Git functionality to allow project maintainers to
+specify hooks that a user ought to install and utilize in their development
@@ Documentation/technical/project-configured-hooks.txt (new)
+
+A large motivation for this change is providing projects a method to enable
+local checks of code e.g. linting. As documented in
-+https://git-scm.com/docs/gitfaq#_hooks[gitfaq], in some cases, server-side ++linkgit:gitfaq#_hooks[gitfaq], in some cases, server-side
+checks may be more appropriate, especially since developers can always skip
+local hook execution. We think there are still benefits to local checks:
+
@@ Documentation/technical/project-configured-hooks.txt (new)
+hooks to run at least before proposing code for submission to the central
+repository to increase the chances of the code getting accepted.
+
-+In the ideal world, developers and project maintainers use both local and server
-+side checks in their workflow. However, for many smaller projects, this may not
-+be possible: CI may be too expensive to run or configure. The number of local
-+solutions to this use case speaks to this need (see <<prior-art, Prior Art>>).
-+Bringing this natively to Git can give all these developers a well-supported,
-+secure implementation opposed to the fragmentation we see today.
++In the ideal world, developers and project maintainers use both local and
++server-side checks in their workflow. However, for many smaller projects, this
++may not be possible: server-side checks may be too expensive, either monetarily
++or in terms of time investment, to run. The number of local solutions to this
++use case speaks to this need (see <<prior-art, Prior Art>>). Bringing this
++natively to Git can give all these developers a well-supported, secure
++implementation opposed to the fragmentation we see today.
+
+User Goals / Critical User Journeys
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ Documentation/technical/project-configured-hooks.txt (new)
+
+ ** I want to skip hooks from running (for various reasons).
+
-+Security Considerations and Design Principles
-+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
++Security Threat Model
++~~~~~~~~~~~~~~~~~~~~~
++
++==== Principles
++
++1. It is unreasonable to require users to audit hook code themselves.
++
++ * Users do not have time
++ * Users, in general, are unfamiliar with system exploits / security
++ * Users may be unfamiliar with a hook’s implementation language
++ * Hooks may be compiled binaries and unauditable
++
++2. It is reasonable for users to make a trust decision based on where a hook comes from.
++
++ * Society functions on the basis of trust relationships formed between different
++ agents --- this is the basis for being able to accomplish more than one person
++ can personally accomplish. The complexity is giving users the power to make that
++ trust decision.
++ * Git users are already making trust decisions based on repository origin.
++
++==== Happy Path
++
++* User clones repository
++
++ ** Clone occurs over a secure transport (HTTPS / SSH)
++
++* After clone, repository suggests hooks to the user
++
++ ** User receives a non-interactive message advertising hooks available to
++ install
++
++ ** User can see what hooks and commands are being suggested and from what
++ remote.
++
++* User evaluates the hook's purpose and publisher and decides whether to install
++it
++
++ ** Installation happens through a separate command the user runs, not an
++ interactive prompt.
++
++* [Minimum Feature] A repository owner can update a hook, at which point users who have
++installed it can decide whether to upgrade or uninstall it
++
++ ** This avoids having to define in the first round of this feature whether
++ the hook's purpose and publisher have changed little enough to reuse the
++ user's previous consent.
+
-+We must balance the desire to make hooks setup easy for developers --- allowing
-+them to get hooks set up with low friction, and hence increasing the probability
-+of them adopting these hooks --- with protecting users from the security risks
-+of arbitrary code execution on their hosts.
++ ** A good enhancement soon after would be to allow users to indicate at hook
++ installation time that they want to accept all updates of this same hook
++ from the same origin.
+
-+To inform the design, we propose these design principles:
++==== In-Scope Threats
+
-+* User consent: Users must explicitly agree to hooks usage; no hooks should
-+execute without consent, and users should re-consent if hooks update. Users can
-+opt-out of hooks.
++1. Repository owner changes hooks frequently to coax user into accidentally or
++mindlessly agreeing to install malicious hooks (“please make this prompt go
++away,” “why are you bothering me?”). Mitigation:
+
-+* Trust comes from the central repository:
-+ ** Most users don't have the time or expertise to properly audit every hook
-+ and what it does. There must be trust between the user and the remote that the
-+ code came from, and the Git project should ensure trust to the degree it can
-+ e.g. enforce HTTPS for its integrity guarantees.
++ ** Never interactively prompt for hook installation
+
-+ ** Since developers will likely build their local clone in their development
-+ process, at some point, arbitrary code from the repository will be executed.
-+ In this sense, hooks _with user consent_ do not introduce a new attack surface.
++ ** Advertise hooks up front instead of waiting until hooks are about to
++ execute. Up front, the user's intent is to set up their workspace so the
++ install message is relevant to them; when the user is in the middle of
++ another action, they are more likely to blindly agree.
+
-+* Give users visibility: Git must allow users to make informed decisions. This
-+means surfacing essential information to the user in a visible manner e.g. what
-+remotes the hooks are coming from, whether the hooks have changed in the latest
-+checkout.
++ ** Give users an easy way to disable all advertisements for hooks
++
++2. Adversary intercepts requests for a repository and inserts malicious hooks
++(person-in-the-middle-type attacks). Mitigation:
++
++ ** Only receive hooks suggestions over HTTPS / SSH
++
++ ** Include the hook origin (domain name) in the command to install a hook
++
++3. Malicious repository owner convinces user, who does not intend to run any
++other git commands in the repository after clone, to install hooks. Mitigation:
++
++ ** Do not automatically install hooks on a normal git clone
++
++ ** Do not interactively prompt to install hooks after the clone is finished.
++
++4. Adversary exploits an old security issue with a hook that has since been
++patched. Mitigation:
++
++ ** Allow users to opt-into hooks getting auto-updated
++
++==== Out-of-Scope Threats
++
++1. Repository owner convinces a user who already intends to build code in the
++repository to knowingly and explicitly agree to install hooks. Justification:
++
++ ** This is equivalent (in terms of attack surface) to setting up your
++ dependencies / building the source code, and thus is not a new attack
++ surface.
++
++2. Typosquatting - user clones from a repository they think they trust but it
++isn’t really the right one. Justification:
++
++ ** Existing problem
++
++3. https://github.com/greatsuspender/thegreatsuspender/issues/1263[Malicious new
++maintainer] of the project creates malicious hooks. Justification:
++
++ ** Similar problem to #1 - maintainer could have instead updated build
++ files.
++
++ ** In the case for auto-updating hooks, updated hooks wouldn’t execute until
++ the user has issued commands in the repository.
+
+Feature Requirements
+~~~~~~~~~~~~~~~~~~~~
@@ Documentation/technical/project-configured-hooks.txt (new)
+Minimum Feature Set
+^^^^^^^^^^^^^^^^^^^
+
-+* A repository can specify a configuration for what hook commands are
-+enabled for what hook events
++* A central repository can specify a recommended configuration for what hook
++commands are enabled for what hook events
++
++ * The suggested configuration should be equivalent to what a user could
++ manually install using config-based hooks.
+
-+* A repository can specify, in this configuration, where the hook
-+commands reside
++* The configuration specifies where the hook commands reside
+
+ ** This could be a path to a script/binary within the repository
+
@@ Documentation/technical/project-configured-hooks.txt (new)
+ ** This could be a user installed command or script/binary that exists
+ outside of the repository and is present in $PATH
+
-+* This configuration should only apply if it was received over HTTPS
++* The configuration should only apply if it was received over a secure channel
++(HTTPS, SSH)
+
-+* A setup command for users to set up hooks
++* The configuration lives outside the worktree.
+
-+ ** Hook setup could happen at clone time assuming the user has consented
-+ e.g. if --setup-hooks is passed to git clone
++ ** Allows updated hooks to apply across history and branches, reducing
++ the maintenance burden of keeping hooks updated.
+
-+* Users must explicitly approve hooks at least once
++ ** Allows different remotes to specify different configuration. This is
++ useful for public vs private central repositories or for repositories which
++ support different Git functionality.
+
-+ ** Running the setup command should count as approval, including if the user
-+ consented during the clone
++* The user receives advice to install hooks.
+
-+ ** When a hook command changes, a user should re-approve execution (note:
-+ implementation should not interfere with requirement listed in “Fast
-+ Follows")
++ ** The advice should clearly indicate the suggested hook command(s) and hook
++ event(s) as well as the central repository that is suggesting them (via
++ remote URL).
+
-+* Automation is able to continue to use clone and other commands
-+non-interactively
++ ** The user should be able to use configuration to turn off this advice.
++
++ ** The advice should appear at the following times:
++
++ *** After a clone
++
++ *** After a suggested hook would have run if not already installed. The
++ advice should include commands for installing the hook and invoking it.
++ For example, for a hook on 'git commit', the user should receive advice
++ to amend their commit after hook installation.
++
++* If, after fetch, the central repository suggests new or updated hooks, the
++user should receive advice to install these new hooks (note: implementation
++should not interfere with requirement listed in“Fast Follows")
+
+* Works across Windows/Linux/macOS
+
+Fast Follows
+^^^^^^^^^^^^
+
-+* When prompted to execute a hook, users can specify always or never, even if
-+the hook updates
++* Behind configuration, a user can opt to automatically install hook updates
++from a given remote.
+
+Nice to Haves
+^^^^^^^^^^^^^
@@ Documentation/technical/project-configured-hooks.txt (new)
+
+* Ensuring the user has installed software that isn't distributed with the repo
+
-+Implementation Exploration: Check "magic" branch for configs at fetch time
-+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-+
+Example User Experience
-+^^^^^^^^^^^^^^^^^^^^^^^
-+
-+===== Case 1: Consent through clone
-+
-+....
-+$ git clone --setup-hooks
-+...
-+
-+The following hooks were installed from remote origin ($ORIGIN_URL):
++~~~~~~~~~~~~~~~~~~~~~~~
+
-+pre-commit: git-secrets --pre_commit_hook
-+pre-push: $GIT_ROOT/pre_push.sh
-+....
-+
-+===== Case 2: Prompting after clone
++==== Example 1: Suggestion to install after clone
+....
-+$ git clone
++$ git clone $ORIGIN_URL
+...
+
-+Remote origin ($ORIGIN_URL) suggest installing the following hooks:
-+
++Remote origin ($ORIGIN_URL) suggests the following hooks:
+pre-commit: git-secrets --pre_commit_hook
+pre-push: $GIT_ROOT/pre_push.sh
+
-+# instead of prompting, we could give users commands to run instead
-+# see case 3
-+
-+Do you wish to install them?
-+1. Yes (this time)
-+2. Yes (always from origin)
-+3. No (not this time)
-+4. No (never)
++If you wish to install them, run git hook setup origin.
++To always ignore hooks from origin, run git hook ignore origin.
+....
+
-+===== Case 3: Re-prompting when hooks change
++==== Example 2: Altering when suggested hooks change
+....
-+$ git pull
++$ git pull origin
+
+The following hooks were updated from remote origin ($ORIGIN_URL):
+
+pre-push: $GIT_ROOT/pre_push.sh
+
+If you wish to install them, run git hook setup origin.
-+
-+If you wish to always accept hooks from origin, run `git hook setup --always
-+origin`. You should only do this if you trust code changes from origin.
-+
+To always ignore hooks from origin, run git hook ignore origin.
+....
+
-+===== Case 4: Nudging when hooks weren't installed
++==== Example 3: Nudging when hooks weren't installed
+....
+$ git commit
-+advice: The repository owner has recommended a 'pre-commit' hook that was not run.
-+To view it, run git show origin/refs/recommended-config:some-pre-commit. To install it, run git hook setup origin pre-commit ++
++advice: The repository owner has recommended a 'pre-commit' hook that was not
++run: git-secrets --pre_commit_hook.
++
++To install it, run git hook setup origin pre-commit. After installation, apply
++the hook to your commit by running git commit --amend.
+
+Turn off this advice by setting config variable advice.missingHook to false."
+....
+
++==== Example 4: Prompting for automatic hook updates, if opted in via config
++....
++#~/.gitconfig
++[hook]
++ allowPromptingAutoUpdateFromRemote = $ORIGIN_URL
+
-+Implementation Sketch
-+^^^^^^^^^^^^^^^^^^^^^
++$ git pull origin
+
-+* Perform fetch as normal
++The following hooks were updated from remote origin ($ORIGIN_URL):
+
-+* After fetch is complete, Git checks for a "magic" config branch (e.g. -++origin/refs/recommended-config+) which contains information about config lines
-+an end-user may want (including hooks).
++pre-push: $GIT_ROOT/pre_push.sh
+
-+* As part of the fetch subcommand, Git prompts users to install the configs
-+contained there.
++If you wish to install them, run git hook setup origin.
+
-+ ** User responses to that prompt could be "sticky" - e.g. a user could reply
-+ "no (this time)", "no (never)", "yes (this time)", or "yes (always)".
-+ Always/never indicate that the user trusts the remote this config is coming
-+ from, and should not apply to configs fetched from other remotes.
++# If and only if the config is set, following only messages shows and the
++# "always" flag works
+
-+Later, we might want to do this before the initial clone is performed; that
-+workflow looks like:
++If you wish to always accept hooks from origin, run `git hook setup origin
++--always`. You should only do this if you trust code changes from origin.
+
-+* During clone, perform ls-refs as normal
++To always ignore hooks from origin, run git hook ignore origin.
++....
+
-+* If the server has a "magic" config branch, fetch only that config branch.
++Implementation Exploration: Check special config branch at fetch time
++~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
-+* Prompt users as described above.
++* Perform fetch as normal
+
-+* Perform the rest of the clone.
++* After fetch is complete, Git checks for a "magic" config branch (e.g. +++origin/refs/recommended-config+) which contains information about config lines
++an end-user may want, including hooks.
++
++* As part of the fetch subcommand, Git alerts users (but not interactively) they
++can install install the configs contained there.
+
+Pros
+^^^^
@@ Documentation/technical/project-configured-hooks.txt (new)
+* Repository owners have a method for providing recommended config for
+contributors.
+
-+* Installation flow happens without additional user intervention.
-+
+* Keeping config branch and history separate from code branch and history means
+it is versioned, but not tied to user's checkout.
+
-+* Letting users specify "always" or "never" reduces amount of pain introduced by
-+interactive "configuration wizard" flow.
-+
+Cons
+^^^^
+
-+* Requires addition of step to fetch (and later clone).
-+
-+* Turning a "set and forget" command like clone into an interactive session with
-+the user is not ideal; care must be taken to avoid breaking bots.
++* Requires addition of step to fetch.
+
+* Inflating configs and executables from a remote tracking branch which is never
+checked out could be slow.
@@ Documentation/technical/project-configured-hooks.txt (new)
+Prior Art
+~~~~~~~~~
+
-+Husky
-+^^^^^
++https://github.com/typicode/husky[Husky]
++^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
++
++* node.js ecosystem-based solution
+
+* Add it as a dev dependency in package.json
+
@@ Documentation/technical/project-configured-hooks.txt (new)
+package.json with husky install will automate the installation of the husky
+script in the .git directory.
+
-+pre-commit
-+^^^^^^^^^^
++https://github.com/pre-commit/pre-commit[pre-commit] ++^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
++
++* Python-based solution
+
+* Acts as a package manager for linting, installing the required linters as
+needed
@@ Documentation/technical/project-configured-hooks.txt (new)
+
+* --ignore-hooks allows developers to run hooks without blocking
+
-+// The '+' in Gerrit URL frightens asciidoctor.
-+:repohook-read: https://android.googlesource.com/platform/tools/repohooks/+/refs/heads/master/rh/config.py
-+:repohook-config: https://android.googlesource.com/platform/tools/repohooks/+/refs/heads/master#config-files
-+
+* The hooks {repohook-read}[read] a {repohook-config}[config file] to determine
+what code to actually execute.
+
@@ Documentation/technical/project-configured-hooks.txt (new)
+ ** commit_msg_bug_field: require “Bug:” to be present in the commit message
+ (built into repohooks)
+
++
++Other Solutions
++^^^^^^^^^^^^^^^
++
++* https://github.com/sds/overcommit - Ruby
++* https://github.com/Arkweid/lefthook - Go
++* https://github.com/shibapm/Komondor - Swift
++
+Glossary
+~~~~~~~~
+
@@ Documentation/technical/project-configured-hooks.txt (new)
+
+*hook command*: A user script or executable which will be run on one or more
+hook events.
+ \ No newline at end of file

 Documentation/Makefile                        |   1 +
 .../technical/project-configured-hooks.txt    | 528 ++++++++++++++++++
 2 files changed, 529 insertions(+)
 create mode 100644 Documentation/technical/project-configured-hooks.txt

index 81d1bf7a049b..06ba5945c428 100644
@@ -96,6 +96,7 @@ TECH_DOCS += technical/protocol-common
 TECH_DOCS += technical/protocol-v2
 TECH_DOCS += technical/racy-git
 TECH_DOCS += technical/reftable
+TECH_DOCS += technical/project-configured-hooks
 TECH_DOCS += technical/send-pack-pipeline
 TECH_DOCS += technical/shallow
 TECH_DOCS += technical/signature-format

base-commit: a5828ae6b52137b913b978e16cd2334482eb4c1f

gitgitgadget

From: Albert Cui <[email protected]>

Hooks today are configured at the repository level, making it difficult to share hooks across repositories. Configuration-based hook management, by moving hooks configuration to the config, makes this much easier. However, there is still no good way for project maintainers to encourage or enforce adoption of specific hook commands on specific hook events in a repository. As such, there are many tools that provide this functionality on top of Git.

This patch documents the requirements we propose for this feature as well as a design sketch for implementation.

---
[RFC] hooks: propose project configured hooks

V4:

* Streamline local vs server-side checks section and better detail this workflow
* Talk about GPG signing as an alternative way to create trust
* Add contributors bypassing local hooks to threat model

[email protected]

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-908%2Falbertcui%2Fhooks-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-908/albertcui/hooks-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/908

Range-diff vs v3:

1: e8fbcc060c71 ! 1: 0bdf84f277b9 hooks: propose project configured hooks
@@ Documentation/technical/project-configured-hooks.txt (new)
+Context
+^^^^^^^
+
-+Git has linkgit:githooks[hooks] functionality to allow users to
-+execute commands or scripts when certain Git events occur. Some use cases -+include:
++Git has linkgit:githooks[hooks] functionality that allows users and ++administrators to run tools or scripts when certain Git events occur. This ++enables workflows that define code style/quality and governance for a project, ++spanning both the client and the server:
+
+* The pre-commit hook event:
+
-+ ** A developer may want to lint their changes to enforce certain code style -+ and quality. The project may want the commit to fail so that commits that
-+ violate the project's style don't get uploaded.
++ ** A developer may want to lint their changes to adapt the project's code
++ style. The project may want the commit to fail so that commits that violate ++ the project's style don't get uploaded.
+
+ ** The project may want to prevent developers from committing passwords or
+ other sensitive information e.g. via
+ https://github.com/awslabs/git-secrets[git-secrets].
+
-+* The commit-msg hook event: the project may want to enforce a certain commit -+message style. This may be out of necessity:
-+https://www.gerritcodereview.com/[Gerrit Code Review], for example, requires -+each commit to have a Change-Id in the footer.
++* The commit-msg hook event: the project may require a certain commit message ++style. This may be out of necessity: https://www.gerritcodereview.com/[Gerrit
++Code Review], for example, requires each commit to have a Change-Id in the ++footer.
+
+* The pre-push hook: the project may want to verify that pushes are going to +the correct central repository to prevent leaks.
+
-+A common thread between these use cases is to enforce certain behavior across -+many developers working in the same code base, encouraging best practices and -+healthy code quality.
++* Finally, the pre-receive hook can be used to enforce the aforementioned ++checks on the server, rejecting any pushes which fail to pass.
+
-+Hooks today are configured individually in each clone, making it difficult for -+project maintainers to enforce hooks usage across them.
++Client-side hooks today are configured individually in each clone, making it ++difficult for project maintainers to define hooks usage across them.
+https://lore.kernel.org/git/[email protected][Configuration-based
-+hook management], by moving hooks to the config, makes this easier; individuals -+can at least configure hooks to be used across multiple workspaces on their -+host.
++hook management], by moving hooks to the Git configuration, makes this easier; ++individuals can at least configure hooks to be used across multiple workspaces ++on their host.
+
+However, there is still no good way for project maintainers to encourage or +enforce adoption of specific hook commands on specific hook events in a clone.
@@ Documentation/technical/project-configured-hooks.txt (new)
+specify hooks that a user ought to install and utilize in their development +workflows.
+
-+Server-side vs Local Checks
-+^^^^^^^^^^^^^^^^^^^^^^^^^^^
-+
-+A large motivation for this change is providing projects a method to enable -+local checks of code e.g. linting. As documented in -+linkgit:gitfaq#_hooks[gitfaq], in some cases, server-side
-+checks may be more appropriate, especially since developers can always skip -+local hook execution. We think there are still benefits to local checks:
-+
-+* Ensuring commit message style and preventing the committing of sensitive data, -+as described above. In particular, if CI results are public, as with many open -+source projects, server side checks are useless for guarding against leaking -+sensitive data.
-+
-+* Helps developers catch issues earlier: typically developers need to push to -+the remote to trigger server-side checks. Local hooks can be run anytime the -+developer wants. This is especially useful if the project has slow -+server-checks; catching issues locally can save the developer a lot of time -+waiting for CI. They are also useful for locally reproducing an issue identified -+in CI, helping resolve issues faster.
-+
-+* Since the typical goal of developers to submit changes to a central -+repository, their interests are aligned with the project maintainers'; while -+they can choose to skip local hook execution, it is in their interest to allow -+hooks to run at least before proposing code for submission to the central -+repository to increase the chances of the code getting accepted.
-+
-+In the ideal world, developers and project maintainers use both local and -+server-side checks in their workflow. However, for many smaller projects, this -+may not be possible: server-side checks may be too expensive, either monetarily -+or in terms of time investment, to run. The number of local solutions to this -+use case speaks to this need (see <<prior-art, Prior Art>>). Bringing this -+natively to Git can give all these developers a well-supported, secure -+implementation opposed to the fragmentation we see today.
++Client-side and Server-side Checks
++^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
++
++As documented in linkgit:gitfaq#_hooks[gitfaq], server-side checks are often ++more appropriate than client-side checks, especially since developers are always ++free to skip local hook execution.
++
++Local checks, however, help developers catch issues earlier: typically, ++developers need to push to the remote to trigger server-side checks. Local hooks ++can be run anytime the developer wants. This is especially useful if the project ++has slow server-checks; catching issues locally can save the developer a lot of ++time waiting for server runs. They are also useful for locally reproducing an ++issue identified by the server.
++
++In an ideal world, developers and project maintainers use both local and ++server-side checks in their workflow. Local checks are best suited for a subset ++of checks that are on the server. The configured checks should be exactly the ++same across these two environments so that results are consistent and ++reproducible. The simplest way to do this would be to configure the same hook ++command locally and on the server.
++
++Local checks are typically configured as pre-commit or pre-push checks, and ++these scripts should run relatively quickly so as to not introduce too much ++friction into a developer's commit workflow (a reason why developers would skip ++these local checks); linting and secrets checking of the staged files is usually ++a good use case opposed to running an entire test suite.
++
++Server-side checks would then run on pre-receive to validate that the users' ++commit passes the checks.
+
+User Goals / Critical User Journeys
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ Documentation/technical/project-configured-hooks.txt (new)
+
+ ** Only receive hooks suggestions over HTTPS / SSH
+
-+ ** Include the hook origin (domain name) in the command to install a hook
++ ** Include the hook origin (domain name) in the command to install a hook.
++ If the configuration came from a signed commit, the signature could also be
++ shown.
+
+3. Malicious repository owner convinces user, who does not intend to run any +other git commands in the repository after clone, to install hooks. Mitigation:
@@ Documentation/technical/project-configured-hooks.txt (new)
+ ** In the case for auto-updating hooks, updated hooks wouldn’t execute until
+ the user has issued commands in the repository.
+
++4. Disobedient contributors bypass local hooks and attempt to push changes that ++go against the project's standards. This could range from the mild issue of ++using different coding style or a more serious issue of checking in credentials ++or user data. Justification:
++ ** Projects should be using server-side checks to require checks, and the
++ addition of this feature doesn't change that. Proper documentation of how we
++ expect users to use this can help prevent users from misusing this feature.
++
+Feature Requirements
+~~~~~~~~~~~~~~~~~~~~
+
@@ Documentation/technical/project-configured-hooks.txt (new)
+* Behind configuration, a user can opt to automatically install hook updates +from a given remote.
+
++* Allow users to make trust decisions based on GPG signing e.g. if the ++configuration came from a signed commit, the signature could be shown along ++with the remote it came from.
++
+Nice to Haves
+^^^^^^^^^^^^^
+
@@ Documentation/technical/project-configured-hooks.txt (new)
+
+*hook command*: A user script or executable which will be run on one or more +hook events.
- \ No newline at end of file

 Documentation/Makefile                        |   1 +
 .../technical/project-configured-hooks.txt    | 537 ++++++++++++++++++
 2 files changed, 538 insertions(+)
 create mode 100644 Documentation/technical/project-configured-hooks.txt

index 81d1bf7a049b..06ba5945c428 100644
@@ -96,6 +96,7 @@ TECH_DOCS += technical/protocol-common
 TECH_DOCS += technical/protocol-v2
 TECH_DOCS += technical/racy-git
 TECH_DOCS += technical/reftable
+TECH_DOCS += technical/project-configured-hooks
 TECH_DOCS += technical/send-pack-pipeline
 TECH_DOCS += technical/shallow
 TECH_DOCS += technical/signature-format

base-commit: a5828ae6b52137b913b978e16cd2334482eb4c1f

gitgitgadget

[snip]

From a user perspective, I think it's better to not just tell users that hooks are available but also /what/ hooks are available.

That said, that doesn't require getting everything from the ref as the hook command itself might be stored in this ref. So something like this seems sufficient to me: "Origin suggests setting up a pre-push hook which runs pre_push.sh. To view the contents of pre_push.sh, run {special command}."

My thought was we'd leverage config hooks and basically write to the config for setting up hooks.

For a concrete example, we use Gerrit for internal reviews and need the Change-Id hook, but we don't want that when upstreaming.

Good question for specifying remotes. This might imply the need for something like git commit --hooks-from=origin.

Good catch, I should clarify that previous point. The main concern is prompting before a hook will execute which would get in the way of existing workflows, making users susceptible to blindly agreeing. Showing advice after the fact doesn't get in the way, and this is one reason why "advice" felt like the right terminology to use (more below): it's merely a helpful message that a user can optionally choose to follow.

Another reason to use "advice" here is because the existing system allows users to turn off the advice when it's not needed for the
requirement: "The user should be able to use configuration to turn off this advice."

Do advice settings work on a per-clone basis? If not, I agree "advice" is probably not the right term.

No, that's what I meant by "Fast Follows" -- not needed for the initial feature.

I agree that on its own, more information upfront is better than less. But we can't store anything more than an object ID (a SHA-1 hash) in a ref, though. Knowing anything beyond the object ID would require fetching some objects, which incurs time and bandwidth (perhaps comparatively a tiny amount in well-behaved repos, but we still need to acknowledge that). (Well, there is also the possibility of using multiple refs to communicate which hooks are present and which are not, but I think that's too cumbersome.)

The config hooks work is still in progress, so I think we need to support both the legacy way and the new way. I think this shouldn't be a problem since both ways involve writing something somewhere.

In any case, if the aim is to support config hooks, I think it's clearer if this section just says "write a config" without explaining what the config can do, since that is the responsibility of the config hooks feature. (But then there remains the question of whether there will be a whitelist of acceptable configs to write.)

I think passing a parameter to choose which hook to invoke is beyond the scope of upstream-supplied hooks. (Also, I don't think that this situation is resolved by having 2 remotes providing 2 different sets of hooks. Perhaps one of the remote's set knows to set the Change-Id trailer, but the other set would not know that it needs to remove a specific trailer.)

Ah, I see. I'm still not sure that it is a good idea. Firstly, the user should have already made this decision when they read the message at clone time, and secondly, this advice might come too late (a pre-commit hook might be "salvageable" but a push hook might not). Having said that, the presence or absence of this doesn't affect the overall design and implementation, so we can leave this in if you want.

Those settings don't work per-clone. My main point, though, is that the user probably would always need to know if there are new hooks.

Ah, maybe call it "future work" then. "Fast follows" is not a term I'm familiar with (and searching online, it doesn't seem like a common term either).