Re: The future of commit access policy for core Firefox

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