Re: merging pull requests

[Kees Cook <keescook@chromium.org>]

On Fri, Oct 01, 2021 at 09:29:14AM +1000, Stephen Rothwell wrote:
> Hi Kees,
> 
> On Thu, 30 Sep 2021 16:09:13 -0700 Kees Cook <keescook@chromium.org> wrote:
> >
> > I guess it depends on the expected order of operations. What do other
> > maintainers do to process a PR? I would think it would be:
> > 
> > - pull remote branch (to FETCH_HEAD)
> > - review (in FETCH_HEAD? in a "real" branch?)
> 
> Our workflow usually requires that all patches are posted to an
> appropriate mailing list for review.  If the developer then also has a
> git branch/tag containing the patch series (after updating for
> Reviewed-bys etc) then that is a separate issue and (probably) requires
> a level of trust on the part of the maintainer (or a recheck) that the
> patches have not been changed since the final review.

Right, that's why I'm such a huge fan of "b4" (and patatt). It makes the
integrity chain very easy to maintain. (Though see my P.S. below...)

> > - merge into my topic branch (where the commit subject should retain details
> >   of the pull location, body has notes from the signed tag, etc...)
> > - review, build, and test
> 
> "rinse and repeat" :-)

Indeed. :)

-Kees

P.S.

The only "hole" I see with the integrity checking is that since only tags
or mbox headers are signed, and those aren't part of the merge, there
isn't a easy way that I see to follow the integrity chain for a given
resulting tree. (Which is technically different from the "trust" chain.)

For example, for stuff going into my tree:
- If it's from an mbox, I can easily check that the patches haven't changed
  in flight when the author used b4/patatt to wrap the email delivery.
- If it's from a remote tag, I can check the tag signature.
This is all fine.

Now I publish my tree, and sign a tag for it for a pull request. Whoever
does that pull can only check my tag and has to trust I checked what
went into my tree. At the end of the day, that's exactly what the
tag signature is for: whoever is pulling must trust the PR sender for
all kinds of reasons. But there isn't a way to mechanically perform an
integrity check on the components of those results: the merged mbox with
the signature headers or the remote tag signature aren't associated
with the resulting branch any more.

But given that maintainers may tweak what was sent to them or squash
fixes, there's likely no point in that kind of integrity chain...

-- 
Kees Cook