Modified: trunk/Tools/ChangeLog (233057 => 233058)
--- trunk/Tools/ChangeLog 2018-06-21 20:58:52 UTC (rev 233057)
+++ trunk/Tools/ChangeLog 2018-06-21 21:11:28 UTC (rev 233058)
@@ -1,3 +1,29 @@
+2018-06-21 Daniel Bates <[email protected]>
+
+ EWS should not try to post comments or upload result archives to security-sensitive
+ bugs unless it has access
+ https://bugs.webkit.org/show_bug.cgi?id=186831
+
+ Reviewed by Lucas Forschler.
+
+ Following r232979 security-sensitive patches are uploaded to the status server so
+ that they can be retrieved and processed by EWS bots without the need for Bugzilla
+ security bug access. Although the EWS machinery is robust against unexpected exceptions,
+ including exceptions raised when interacting with Bugzilla bugs/attachments with
+ insufficient credentials, we should not depend on such defenses as they cause webkit-
+ patch to log a message for the "unexpected" exception. We should reserve such logging
+ for truly unexpected exceptions that indicate a programming mistake that we need to fix.
+
+ * Scripts/webkitpy/tool/commands/earlywarningsystem.py:
+ (AbstractEarlyWarningSystem._post_reject_message_on_bug): Bail out early if we cannot
+ access the bug.
+ * Scripts/webkitpy/tool/commands/queues.py:
+ (PatchProcessingQueue._can_access_bug): Added.
+ (PatchProcessingQueue._upload_results_archive_for_patch): Only add an attachment if we
+ can access the bug.
+ (CommitQueue.process_work_item): Only post a rejection comment (i.e. call CommitterValidatorreject_patch_from_commit_queue())
+ if we can access the bug.
+
2018-06-21 Lucas Forschler <[email protected]>
bisect-builds --list not showing all builds
Modified: trunk/Tools/Scripts/webkitpy/tool/commands/earlywarningsystem.py (233057 => 233058)
--- trunk/Tools/Scripts/webkitpy/tool/commands/earlywarningsystem.py 2018-06-21 20:58:52 UTC (rev 233057)
+++ trunk/Tools/Scripts/webkitpy/tool/commands/earlywarningsystem.py 2018-06-21 21:11:28 UTC (rev 233058)
@@ -89,6 +89,8 @@
message += "\n\n%s" % extra_message_text
# FIXME: We might want to add some text about rejecting from the commit-queue in
# the case where patch.commit_queue() isn't already set to '-'.
+ if not self._can_access_bug(patch.bug_id()):
+ return
if self.watchers:
tool.bugs.add_cc_to_bug(patch.bug_id(), self.watchers)
tool.bugs.set_flag_on_attachment(patch.id(), "commit-queue", "-", message)
Modified: trunk/Tools/Scripts/webkitpy/tool/commands/queues.py (233057 => 233058)
--- trunk/Tools/Scripts/webkitpy/tool/commands/queues.py 2018-06-21 20:58:52 UTC (rev 233057)
+++ trunk/Tools/Scripts/webkitpy/tool/commands/queues.py 2018-06-21 21:11:28 UTC (rev 233058)
@@ -299,6 +299,13 @@
self._create_port()
+ # FIXME: Bugzilla member functions should perform this check as they can do it as part of the same
+ # network request. This would also avoid bugs where the access of the Bugzilla bug changes between
+ # the time-of-check (calling this function) and time-of-use (when we ask Bugzilla to perform the
+ # actual operation).
+ def _can_access_bug(self, bug_id):
+ return bool(self._tool.bugs.fetch_bug(bug_id))
+
def _upload_results_archive_for_patch(self, patch, results_archive_zip):
if not self._port:
self._create_port()
@@ -316,7 +323,8 @@
# FIXME: We could easily list the test failures from the archive here,
# currently callers do that separately.
comment_text += BotInfo(self._tool, self._port.name()).summary_text()
- self._tool.bugs.add_attachment_to_bug(patch.bug_id(), results_archive_file, description, filename="layout-test-results.zip", comment_text=comment_text)
+ if self._can_access_bug(patch.bug_id()):
+ self._tool.bugs.add_attachment_to_bug(patch.bug_id(), results_archive_file, description, filename="layout-test-results.zip", comment_text=comment_text)
class CommitQueue(PatchProcessingQueue, StepSequenceErrorHandler, CommitQueueTaskDelegate):
@@ -350,8 +358,9 @@
self._did_error(patch, "%s did not process patch. Reason: %s" % (self.name, error.failure_message))
return False
except ScriptError as e:
- validator = CommitterValidator(self._tool)
- validator.reject_patch_from_commit_queue(patch.id(), self._error_message_for_bug(task, patch, e))
+ if self._can_access_bug(patch.bug_id()):
+ validator = CommitterValidator(self._tool)
+ validator.reject_patch_from_commit_queue(patch.id(), self._error_message_for_bug(task, patch, e))
results_archive = task.results_archive_from_patch_test_run(patch)
if results_archive:
self._upload_results_archive_for_patch(patch, results_archive)
