Closed Bug 1420510 Opened 7 years ago Closed 5 years ago

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

Categories

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

Product:
Developer Services
Component:
Mercurial: hg.mozilla.org
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: Gijs, Assigned: zeid)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

hook.py
(deleted), text/plain
sheehan
: feedback-
Details
hooks: adding push hook for security bugs (Bug 1420510) r=sheehan
(deleted), text/x-phabricator-request
Details
(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 :-( )
Product: Release Engineering → Developer Services
Component: General → Mercurial: hg.mozilla.org
(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?
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.
Blocks: require-mach-try
Attached file hook.py (deleted) —

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.

Assignee: nobody → tom
Status: NEW → ASSIGNED
Attachment #9112128 - Flags: feedback?(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.
Attachment #9112128 - Flags: feedback?(sheehan) → feedback-

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

Assignee: tom → zeidzabaneh

DO NOT LAND - WORK IN PROGRESS

Attachment #9112559 - Attachment description: hooks: adding push hook for security bugs (Bug 1420510) r=sheehan 50% → hooks: adding push hook for security bugs (Bug 1420510) r=sheehan 80%

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?

Attachment #9112559 - Attachment description: hooks: adding push hook for security bugs (Bug 1420510) r=sheehan 80% → hooks: adding push hook for security bugs (Bug 1420510) r=sheehan 70%
Attachment #9112559 - Attachment description: hooks: adding push hook for security bugs (Bug 1420510) r=sheehan 70% → hooks: adding push hook for security bugs (Bug 1420510) r=sheehan 80%
Attachment #9112559 - Attachment description: hooks: adding push hook for security bugs (Bug 1420510) r=sheehan 80% → hooks: adding push hook for security bugs (Bug 1420510) r=sheehan 85%

NI for comment 7.

Flags: needinfo?(zeidzabaneh)

(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.

Flags: needinfo?(zeidzabaneh)
Attachment #9112559 - Attachment description: hooks: adding push hook for security bugs (Bug 1420510) r=sheehan 85% → hooks: adding push hook for security bugs (Bug 1420510) r=sheehan 90%
Attachment #9112559 - Attachment description: hooks: adding push hook for security bugs (Bug 1420510) r=sheehan 90% → hooks: adding push hook for security bugs (Bug 1420510) r=sheehan 95%
Attachment #9112559 - Attachment description: hooks: adding push hook for security bugs (Bug 1420510) r=sheehan 95% → hooks: adding push hook for security bugs (Bug 1420510) r=sheehan 97%
Attachment #9112559 - Attachment description: hooks: adding push hook for security bugs (Bug 1420510) r=sheehan 97% → hooks: adding push hook for security bugs (Bug 1420510) r=sheehan 99%
Attachment #9112559 - Attachment description: hooks: adding push hook for security bugs (Bug 1420510) r=sheehan 99% → hooks: adding push hook for security bugs (Bug 1420510) r=sheehan

Pushed by cosheehan@mozilla.com:
https://hg.mozilla.org/hgcustom/version-control-tools/rev/0471a6491e0e
hooks: adding push hook for security bugs r=sheehan

Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED

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

Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → RESOLVED
Closed: 5 years ago → 5 years ago
Resolution: --- → FIXED
Regressions: 1605656
Regressions: 1607549
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: