Re: The future of commit access policy for core Firefox

Top-posting because I'm not really responding to Dan.

I'd like to suggest we take a step back here from the technical mechanisms
and ask
a simpler policy question, namely:

"What is the minimum number of 'trusted' people that should be required to
land
a patch"?

Currently, I suspect the answer is effectively 0: a random person can post
a patch to
Bugzilla, mark it "r=<committer>" (with some claim that it was in IRC) and
then mark
it checkin-needed and (I suspect, but perhaps the Sheriffs will correct me)
it will get
landed. Alternately, you can create a patch which gets r+ with nits, and
then update
with some malicious code and  r=<committer>.

So, I would ask: do people believe that this is an acceptable state of
affairs or
should the minimum number of 'trusted' people required to land a patch be 1
or
more?

-Ekr









On Sun, Mar 12, 2017 at 5:44 PM, Daniel Veditz <[email protected]> wrote:

> On 3/12/17 6:54 AM, Trevor Saunders wrote:
> > If we pretend the goal has no downside then of course it seems like a
> > good idea.  However its really hard for me to imagine a world where its
> > worth the time to review patches that amount to
> >
> > +  MOZ_ASSERT(x);
> >
> > after I asked you to please assert x there.  The worst downsides would
> > seem to be forgeting to add the assert which often isn't that big a
> > deal, or you adding a wrong assert which will show up soon enough.
>
> That assumes the developers you review have good intentions--which is
> almost always true. Like most security-inspired restrictions, however,
> the worry is about someone with bad intentions. It's similar to
> insurance in that you pay every year but really you hope that your house
> does NOT burn down, but in that case it feels like a waste.
>
> Hypothetically, an evil person could take a "r+ with nits" and then
> check in the patch with bad code that was never seen by another human
> and which may not get noticed. When we put autoland into the mix this
> essentially gives L3 commit access to anyone who can put together a
> helpful "almost right" patch. I think there's not a lot of controversy
> about imposing this restriction for patch submitters who are not yet
> trusted.
>
> This does add overhead when applied to developers who are, in fact,
> trusted. On the surface, clearly noted in this thread, that seems
> insane. But at the top of this thread, the worry was "what if a trusted
> committer's credentials get compromised?" Currently the answer is "We're
> screwed". We can hope the sheriffs would notice an odd patch during
> uplift, but the sheriffs are busy and any attacker who was any good
> would make sure the patch looked totally normal on the surface.
>
> No easy answers.
>
> -Dan Veditz
> _______________________________________________
> dev-planning mailing list
> [email protected]
> https://lists.mozilla.org/listinfo/dev-planning
>