Re: [RFC PATCH 0/2] Add named reference to latest push cert

From: Stefan Beller <sbeller@google.com>
Date: Sep 7, 2017, 17:43 UTC
Message-ID: <CAGZ79kZO_u21JeAsNq7p7X88i0wmP6kJcg=JbKp62Bxdm4d4Uw@mail.gmail.com>
In-Reply-To: <20170907091133.zygswf3f6z2bedfo@weakknees.security.iitk.ac.in>

On Thu, Sep 7, 2017 at 2:11 AM, Shikher Verma <root@shikherverma.com> wrote:

Show 25 quoted lines

> On Wed, Sep 06, 2017 at 02:31:49PM -0700, Stefan Beller wrote:
>> On Wed, Sep 6, 2017 at 2:39 AM, Shikher Verma <root@shikherverma.com> wrote:
>> > Currently, git only stores push certificates if there is a receive hook
>> > present. This may violate the principle of least surprise (e.g., I
>> > pushed with --signed, and I don't see anything in upstream).
>> > Additionally, push certificates could be more versatile if they are not
>> > tightly bound to git hooks. Finally, it would be useful to verify the
>> > signed pushes at later points of time with ease.
>> >
>> > A named ref is added for ease of access/tooling around push
>> > certificates. If the last push was signed, ref/PUSH_CERT stores the
>> > ref of the latest push cert otherwise it is empty.
>> >
>> > Sending patches as RFC since the documentation would have to be
>> > updated and git gc might have to be patched to not garbage collect
>> > the latest push certificate.
>> >
>> > This patch applies on master (3ec7d702a)
>>
>> What are performance implications for busy repositories at busy hosts?
>> (think kernel.org / github) They may want to disable this new feature
>> for performance reasons or because they don't want to clutter the
>> object store. So at least a config option to turn it off would be useful.
>
> Any typical git push would write several objects to disk,

(or just one pack file, [which may be renamed eventually, see 722ff7f8])

> this patch
> would only add one more object per push so I think the performance
> penalty is not that high. But I agree that we can have a config to turn
> it off.

I personally do not run a high performance server, so I do not terribly mind, but thought it would be nice for them to have at least an option ready made instead of a potential performance regression.

Show 22 quoted lines

>> On the ref to store the push certs:
>> (a) Currently the ref points at the blob, I wonder if we'd rather want to
>> point at a commit? (Then we can build up a history of
>> push certs, instead of relying on the reflog to show all
>> push certs. Also the gc issue might be easier to solve using this)
>
> I am not sure how that would work. The ref points at the blob of push
> certificate. Since each push can update multiple refs, each push
> certificate can point to mutiple commits (tip of the updated refs).
> Also if the named ref points at the commit then how will we get the
> corresponding push certificate?
>
> I did think about keeping a history of push certificates but the problem
> is new pushes can delete refs and commits which were pointed to by
> previous push certificates. This makes it really difficult to decide
> which push certificates to keep and which to gc. Also this history would
> be different for different clones of the same repo. Since push
> certificate are only meta data of the git workflow I think its best to
> just keep the latest push certificate and gc the old ones. People can
> use the recieve hook if they want to do advance things like logging a
> history of push certificates. I think git should provide a builtin
> solution for the simple case.

What I had in mind was what would be achieved with a hook like this (untested):

    #!/bin/sh
    if test -z GIT_PUSH_CERT ; then
        exit 0
    fi

    # add a new worktree 'tmp', checking out the magic ref:
    git worktree add tmp refs/PUSH_CERT

    cp $GIT_PUSH_CERT tmp/cert
    git -C tmp add cert
    git -C tmp commit -m "new push cert"
    # maybe include GIT_PUSH_CERT_[NONCE_]STATUS
    # in commit message?

    # clean up, command doesn't exist yet:
    git worktree delete tmp

This would not deal with concurrency as it re-uses the same worktree, but illustrates what I had in mind for the git history of that special ref.