git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Albert Cui <albertqcui@gmail.com>
To: Emily Shaffer <emilyshaffer@google.com>
Cc: "Albert Cui via GitGitGadget" <gitgitgadget@gmail.com>,
	git@vger.kernel.org,
	"brian m. carlson" <sandals@crustytoothpaste.net>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: Re: [PATCH v2] hooks: propose project configured hooks
Date: Thu, 1 Apr 2021 13:02:24 -0700	[thread overview]
Message-ID: <CAMbkP-T4xUNb2SyXPic_XcJXUNGa2kKTADdTJ45-e+rw8aNa5g@mail.gmail.com> (raw)
In-Reply-To: <YGJgw5QPKFyv4HSG@google.com>

On Mon, Mar 29, 2021 at 4:20 PM Emily Shaffer <emilyshaffer@google.com> wrote:
>
> > +Security Considerations and Design Principles
> > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > +
> [snip]
> > +  ** 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.
>
> It might be worth saying that we want to make configuration of
> project-configured hooks to be approximately as easy/automatic as
> building (that is, the user still has to explicitly run a build, and
> isn't prompted at the end of their clone whether they want to build it
> right away).

+1, I like phrasing it this way.
> > +
> > +* 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.
>    ^~~~~~~~
> Better say "fetch", if we are proposing this magic branch thing.
>
> > +* This configuration should only apply if it was received over HTTPS
>
> Meaning, non-HTTPS fetches should just not update this special branch?

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

> > +* A setup command for users to set up hooks
> 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
>
Correct, I think `git hook` is a natural evolution. This is a nice to
have that we can document.

>
> > +Fast Follows
> > +^^^^^^^^^^^^
> > +
> > +* When prompted to execute a hook, users can specify always or never, even if
> > +the hook updates
>
> I think we want to base this on the remote URL, right? I know we talked
> a little offline about how to mitigate vs. malicious maintainer (for
> example this whole mess with The Great Suspender) and I'm not sure what
> solution there might be.
>
> I wonder if it's worth it to notify users that their always-okayed hooks
> were updated during fetch?
>
It definitely aligns with the security principles to notify, even if
they have OK'd updates.

> > +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
>
> Hm, I thought we wanted to consider storing the hook body in the magic
> branch as well? To avoid changing hook implementation during bisect, for
> example?
>

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.

  reply	other threads:[~2021-04-01 20:02 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-18 22:03 [PATCH] hooks: propose repository owner " Albert Cui via GitGitGadget
2021-03-18 22:29 ` Junio C Hamano
2021-03-18 23:45   ` Albert Cui
2021-03-19  1:28 ` brian m. carlson
2021-03-19 10:27 ` Ævar Arnfjörð Bjarmason
2021-04-06  0:35   ` Albert Cui
2021-04-07 22:47     ` Ævar Arnfjörð Bjarmason
2021-06-21 19:36       ` Jonathan Tan
2021-06-21 20:35         ` Ævar Arnfjörð Bjarmason
2021-03-26  1:43 ` [PATCH v2] hooks: propose project " Albert Cui via GitGitGadget
2021-03-29 23:20   ` Emily Shaffer
2021-04-01 20:02     ` Albert Cui [this message]
2021-03-30 15:24   ` Derrick Stolee
2021-04-05 22:45     ` Albert Cui
2021-04-05 23:09       ` Junio C Hamano
2021-04-05 23:40         ` Albert Cui
2021-04-06  0:13           ` Junio C Hamano
2021-04-06  0:27             ` Albert Cui
2021-04-06 23:15       ` brian m. carlson
2021-04-07  7:53         ` Ævar Arnfjörð Bjarmason
2021-04-07 13:09           ` Derrick Stolee
2021-04-07 18:40             ` Albert Cui
2021-04-07 20:02               ` Junio C Hamano
2021-04-07 22:23                 ` Ævar Arnfjörð Bjarmason
2021-04-15 16:52             ` Ed Maste
2021-04-15 19:41               ` Junio C Hamano
2021-04-15 20:37                 ` Ed Maste
2021-04-15 20:50                   ` Junio C Hamano
2021-04-15 22:28                   ` brian m. carlson
2021-04-02  9:59   ` Ævar Arnfjörð Bjarmason
2021-04-05 23:42     ` Albert Cui
2021-04-02 10:30   ` Ævar Arnfjörð Bjarmason
2021-04-03  0:58     ` Albert Cui
2021-04-24  1:38   ` [PATCH v3] " Albert Cui via GitGitGadget
2021-04-28  2:48     ` Junio C Hamano
2021-05-05 19:11     ` [PATCH v4] " Albert Cui via GitGitGadget
2021-06-03  3:31       ` Jonathan Tan
2021-06-03 20:16         ` Albert Cui
2021-06-03 22:10           ` Jonathan Tan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAMbkP-T4xUNb2SyXPic_XcJXUNGa2kKTADdTJ45-e+rw8aNa5g@mail.gmail.com \
    --to=albertqcui@gmail.com \
    --cc=avarab@gmail.com \
    --cc=emilyshaffer@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=sandals@crustytoothpaste.net \
    --subject='Re: [PATCH v2] hooks: propose project configured hooks' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).