1420510 - Add a push hook to try that rejects pushes that contain bug numbers for security bugs

Status: RESOLVED FIXED

(Developer Services :: Mercurial: hg.mozilla.org, enhancement)

Description

[:Gijs (he/him)]

(probably the wrong prod/comp, please move as appropriate)

If you don't know about https://wiki.mozilla.org/Security/Bug_Approval_Process and this is your first sec bug, and you just wrote a patch, you ask for review and... you push to try. It would be nice if we rejected such pushes before they got published on the server, and:

a) linked to https://wiki.mozilla.org/Security/Bug_Approval_Process
b) provided some TL;DR rationale for blocking the push.

(and obviously, (c) had some way to override this. Not sure what that should be without giving people a trivial way of querying try for "trypushes that are for security bugs :-( )

Comment 1

[:Gijs (he/him)]

(In reply to :Gijs from comment #0)
> (and obviously, (c) had some way to override this. Not sure what that should
> be without giving people a trivial way of querying try for "trypushes that
> are for security bugs :-( )

I mean, the obvious solution here would be to suggest pushing without any bug numbers if people really really need to push to try, I guess? Rewriting commit messages should be relatively straightforward in 2017 hg/git. Hopefully/maybe?

Comment 2

[Gregory Szorc [:gps]]

Since we're working towards requiring `mach try`, we could build this functionality into `mach try`. Then if we ever have "private" Try infrastructure (bug 1136954), `mach try` could redirect to that automatically if the bug summary referenced a private bug, the pushed changesets are private, the user requested it, etc.

Anyway, hooking this up to the "require `mach try`" bug dependencies so we don't lose track of the potential feature.

Comment 3

[Tom Ritter [:tjr] (ni? for response to sec-[advisories/bounties/ratings/cves])]

Attachment: hook.py

I've started work on this and came up with the following. I haven't tested it (I'll need to set up a local mercurial server I think, and I won't get to that until next week) but I think it's reasonably put together enough to request feedback.

Comment 4

[Connor Sheehan [:sheehan]]

Comment on attachment 9112128 [details]
hook.py

This looks pretty good! I'm going to pass this along to Zeid for reference. It uses the old hooks format so we will have to convert to the new one, but the error message and flow looks mostly correct.

Comment 5

[Connor Sheehan [:sheehan]]

Zeid is going to be working on this as his first bug.

Comment 6

[Zeid Zabaneh [:zeid]]

Attachment: hooks: adding push hook for security bugs (Bug 1420510) r=sheehan

DO NOT LAND - WORK IN PROGRESS

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Are we going to be able to differentiate between sec bugs and moco-confidential bugs or is this hook going to treat both the same?

Comment 8

[Ryan VanderMeulen [:RyanVM]]

NI for comment 7.

Comment 9

[Zeid Zabaneh [:zeid]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #8)

NI for comment 7.

As far as I can tell the status code and error code are the same for both types of bugs when trying to access them via the API, so with the current implementation the hook will reject both.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/hgcustom/version-control-tools/rev/0471a6491e0e
hooks: adding push hook for security bugs r=sheehan

Comment 11

[Connor Sheehan [:sheehan]]

This has landed and will be deployed soon. We can close the bug once we've rolled it out successfully.

Comment 12

[Sebastian Hengst [:aryx] (needinfo on intermittent or backout)]

Turned off in bug 1607003.