Details
-
Task
-
Resolution: Unresolved
-
P2: Important
-
None
-
None
-
None
Description
Self approvals are dangerous, since by nature, it means that the final version of the submitted code was not reviewed according to policy. Totally removing self-approvals would be too much of a roadblock to developers with approver rights, as often self-approval is a result of re-approving a change after fixing a few final nit-picks and bothering the original approver is seen to waste time and effort.
Perhaps we can monitor for self-approvals and run some checks to validate that no meaningful code change occurred, or that language of the commit message is not materially different. While difficult to specifically scan for security risky minor changes, we can either block suspicious self-approvals or force a further review of the approval in a non-blocking way within a reasonable period.
- In connection with QUIP-23, self-approvals of security-significant code should be treated as suspicious.
- Require a reason comment for self-approvals. How difficult is this to implement in gerrit plugin?
- Can submit requirements be set with a timer? Self-approval could require a 6hr cool-off period before staging is allowed.
- Once per month, perform a randomized sampling audit of self-approved changes.
- Create a group for users which are known to abuse self-approvals and block the ability.
- Analyze code changes in self-approvals for significance and intent. Consider natural language changes of commit messages.