Bug 1164063 - show a warning near the attachments table for sec-high/sec-crit bugs without sec-approval? on patches

6 files changed, 108 insertions, 19 deletions

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index 312039306..73dc98963 100644
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -3621,6 +3621,12 @@ sub keyword_objects {
     return $self->{'keyword_objects'};
 }
 
+sub has_keyword {
+    my ($self, $keyword) = @_;
+    $keyword = lc($keyword);
+    return any { lc($_->name) eq $keyword } @{ $self->keyword_objects };
+}
+
 sub comments {
     my ($self, $params) = @_;
     return [] if $self->{'error'};
diff --git a/extensions/BMO/Extension.pm b/extensions/BMO/Extension.pm
index f4e16d416..6938eb790 100644
--- a/extensions/BMO/Extension.pm
+++ b/extensions/BMO/Extension.pm
@@ -46,7 +46,7 @@ use DateTime;
 use Email::MIME::ContentType qw(parse_content_type);
 use Encode qw(find_encoding encode_utf8);
 use File::MimeInfo::Magic;
-use List::MoreUtils qw(natatime any);
+use List::MoreUtils qw(natatime any last_value);
 use List::Util qw(first);
 use Scalar::Util qw(blessed);
 use Sys::Syslog qw(:DEFAULT);
@@ -67,6 +67,7 @@ BEGIN {
     *Bugzilla::Bug::reporters_hw_os                 = \&_bug_reporters_hw_os;
     *Bugzilla::Bug::is_unassigned                   = \&_bug_is_unassigned;
     *Bugzilla::Bug::has_current_patch               = \&_bug_has_current_patch;
+    *Bugzilla::Bug::missing_sec_approval            = \&_bug_missing_sec_approval;
     *Bugzilla::Product::default_security_group      = \&_default_security_group;
     *Bugzilla::Product::default_security_group_obj  = \&_default_security_group_obj;
     *Bugzilla::Product::group_always_settable       = \&_group_always_settable;
@@ -821,6 +822,54 @@ sub _bug_has_current_patch {
     return 0;
 }
 
+sub _bug_missing_sec_approval {
+    my ($self) = @_;
+    # see https://wiki.mozilla.org/Security/Bug_Approval_Process for the rules
+
+    # no need to alert once a bug is closed
+    return 0 if $self->resolution;
+
+    # only bugs with sec-high or sec-critical keywords need sec-approval
+    return 0 unless $self->has_keyword('sec-high') || $self->has_keyword('sec-critical');
+
+    # look for patches with sec-approval set to any value
+    foreach my $attachment (@{ $self->attachments }) {
+        next if $attachment->isobsolete || !$attachment->ispatch;
+        foreach my $flag (@{ $attachment->flags }) {
+            # only one patch needs sec-approval
+            return 0 if $flag->name eq 'sec-approval';
+        }
+    }
+
+    # tracking flags
+    require Bugzilla::Extension::TrackingFlags::Flag;
+    my $flags = Bugzilla::Extension::TrackingFlags::Flag->match({
+        product     => $self->product,
+        component   => $self->component,
+        bug_id      => $self->id,
+        is_active   => 1,
+        WHERE       => {
+            'name like ?' => 'cf_status_firefox%',
+        },
+    });
+    # set flags are added after the sql query, filter those out
+    $flags = [ grep { $_->name =~ /^cf_status_firefox/ } @$flags ];
+    return 0 unless @$flags;
+
+    my $nightly = last_value { $_->name !~ /_esr\d+$/ } @$flags;
+    my $set = 0;
+    foreach my $flag (@$flags) {
+        my $value = $flag->bug_flag($self->id)->value;
+        next if $value eq '---';
+        $set++;
+        # sec-approval is required if any of the current status-firefox
+        # tracking flags that aren't the latest are set to 'affected'
+        return 1 if $flag->name ne $nightly->name && $value eq 'affected';
+    }
+    # sec-approval is required if no tracking flags are set
+    return $set == 0;
+}
+
 sub _product_default_platform_id { $_[0]->{default_platform_id} }
 sub _product_default_op_sys_id   { $_[0]->{default_op_sys_id}   }
 
diff --git a/extensions/BMO/template/en/default/hook/attachment/list-warnings.html.tmpl b/extensions/BMO/template/en/default/hook/attachment/list-warnings.html.tmpl
index bc4480084..693051996 100644
--- a/extensions/BMO/template/en/default/hook/attachment/list-warnings.html.tmpl
+++ b/extensions/BMO/template/en/default/hook/attachment/list-warnings.html.tmpl
@@ -7,12 +7,26 @@
   #%]
 
 [%
-  RETURN UNLESS user.in_group('editbugs');
-  RETURN UNLESS bug.attachments.size && bug.is_unassigned && bug.has_current_patch;
+  RETURN UNLESS user.in_group('editbugs') || bug.assigned_to.id == user.id;
+  RETURN UNLESS bug.attachments.size && bug.has_current_patch;
 %]
-<tr class="bz_attach_footer">
-  <td colspan="[% show_attachment_flags ? 3 : 2 %]" id="unassigned_with_patches">
-    <img src="extensions/BMO/web/images/sign_warning.png" width="16" height="16">
-    Unassigned [% terms.bug %] with patches attached
-  </td>
-</tr>
+
+[% IF bug.is_unassigned %]
+  <tr class="bz_attach_footer">
+    <td colspan="[% show_attachment_flags ? 3 : 2 %]" class="attachment-warning">
+      <img src="extensions/BMO/web/images/sign_warning.png" width="16" height="16">
+      Unassigned [% terms.bug %] with patches attached
+    </td>
+  </tr>
+[% END %]
+
+[% IF bug.missing_sec_approval %]
+  <tr class="bz_attach_footer">
+    <td colspan="[% show_attachment_flags ? 3 : 2 %]" id="sec-approval-warning" class="attachment-warning">
+      <img src="extensions/BMO/web/images/sign_warning.png" width="16" height="16">
+      <a href="https://wiki.mozilla.org/Security/Bug_Approval_Process" target="_blank">
+        sec-approval required on patches before landing
+      </a>
+    </td>
+  </tr>
+[% END %]
diff --git a/extensions/BMO/template/en/default/hook/bug_modal/attachments-warnings.html.tmpl b/extensions/BMO/template/en/default/hook/bug_modal/attachments-warnings.html.tmpl
index b00eabc1f..5242f47b4 100644
--- a/extensions/BMO/template/en/default/hook/bug_modal/attachments-warnings.html.tmpl
+++ b/extensions/BMO/template/en/default/hook/bug_modal/attachments-warnings.html.tmpl
@@ -7,10 +7,22 @@
   #%]
 
 [%
-  RETURN UNLESS user.in_group('editbugs');
-  RETURN UNLESS bug.attachments.size && bug.is_unassigned && bug.has_current_patch;
+  RETURN UNLESS user.in_group('editbugs') || bug.assigned_to.id == user.id;
+  RETURN UNLESS bug.attachments.size && bug.has_current_patch;
 %]
-<div id="unassigned_with_patches">
-  <img src="extensions/BMO/web/images/sign_warning.png" width="16" height="16">
-  Unassigned [% terms.bug %] with patches attached
-</div>
+
+[% IF bug.is_unassigned %]
+  <div class="attachment-warning">
+    <img src="extensions/BMO/web/images/sign_warning.png" width="16" height="16">
+    Unassigned [% terms.bug %] with patches attached
+  </div>
+[% END %]
+
+[% IF bug.missing_sec_approval %]
+  <div id="sec-approval-warning" class="attachment-warning">
+    <img src="extensions/BMO/web/images/sign_warning.png" width="16" height="16">
+    <a href="https://wiki.mozilla.org/Security/Bug_Approval_Process" target="_blank">
+      sec-approval required on patches before landing
+    </a>
+  </div>
+[% END %]
diff --git a/extensions/BMO/web/styles/bug_modal.css b/extensions/BMO/web/styles/bug_modal.css
index 3de7bde8b..8d14ed11a 100644
--- a/extensions/BMO/web/styles/bug_modal.css
+++ b/extensions/BMO/web/styles/bug_modal.css
@@ -13,10 +13,14 @@
     border-radius: 4px;
 }
 
-#unassigned_with_patches {
+.attachment-warning {
     padding-left: 4px;
 }
 
-#unassigned_with_patches img {
+.attachment-warning img {
     vertical-align: sub;
 }
+
+#sec-approval-warning a {
+    color: #b70000;
+}
diff --git a/extensions/BMO/web/styles/edit_bug.css b/extensions/BMO/web/styles/edit_bug.css
index 7da05a126..fa4403177 100644
--- a/extensions/BMO/web/styles/edit_bug.css
+++ b/extensions/BMO/web/styles/edit_bug.css
@@ -40,10 +40,14 @@ input#cf_rank {
     width: 3em;
 }
 
-#unassigned_with_patches {
+.attachment-warning {
     font-weight: normal;
 }
 
-#unassigned_with_patches img {
+.attachment-warning img {
     vertical-align: sub;
 }
+
+#sec-approval-warning a {
+    color: #b70000;
+}